| Message ID | 20250503201806.3045723-1-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | meson: use thorough test setup as default | expand |
Il sab 3 mag 2025, 22:18 Pierrick Bouvier <pierrick.bouvier@linaro.org> ha scritto: > Allows all tests to be visible by default when using meson test > directly. > > This has no impact on make check-* commands, which use SPEED=quick by > default (see scripts/mtest2make.py). > What's the advantage of having different defaults depending on whether you use "make check" or "meson test"? I don't oppose this change per se, but if it's useful it should be matched by a change in the default SPEED. Paolo > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > meson.build | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/meson.build b/meson.build > index f8bf6e7bb66..57ff3f722d8 100644 > --- a/meson.build > +++ b/meson.build > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: > true, > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > env: ['RUST_BACKTRACE=1']) > add_test_setup('slow', exclude_suites: ['thorough'], > env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) > add_test_setup('thorough', > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', > 'RUST_BACKTRACE=1']) > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', > 'RUST_BACKTRACE=1'], > + is_default: true) > > meson.add_postconf_script(find_program('scripts/symlink-install-tree.py')) > > -- > 2.47.2 > >
On 03/05/2025 22.18, Pierrick Bouvier wrote: > Allows all tests to be visible by default when using meson test > directly. > > This has no impact on make check-* commands, which use SPEED=quick by > default (see scripts/mtest2make.py). > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > meson.build | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/meson.build b/meson.build > index f8bf6e7bb66..57ff3f722d8 100644 > --- a/meson.build > +++ b/meson.build > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > env: ['RUST_BACKTRACE=1']) > add_test_setup('slow', exclude_suites: ['thorough'], > env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) > add_test_setup('thorough', > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], > + is_default: true) I'd rather not make it the default: The thorough functional tests download a lot of assets from the internet, so if someone just runs "meson test" without further parameters, I think we should not trigger these downloads in that case. Thomas
On 5/4/25 11:00 PM, Paolo Bonzini wrote: > > > Il sab 3 mag 2025, 22:18 Pierrick Bouvier <pierrick.bouvier@linaro.org > <mailto:pierrick.bouvier@linaro.org>> ha scritto: > > Allows all tests to be visible by default when using meson test > directly. > > This has no impact on make check-* commands, which use SPEED=quick by > default (see scripts/mtest2make.py). > > > What's the advantage of having different defaults depending on whether > you use "make check" or "meson test"? > "make check" can be seen as a blackbox, where you have different targets, but don't really do things on a per test basis. At least, it's not the workflow that was implemented. "meson test", on the opposite, expose the list of tests to you. My rationale here is to expose all tests for meson test (instead of having to add --setup thorough everytime), and let the "per setup" workflow to make command. > I don't oppose this change per se, but if it's useful it should be > matched by a change in the default SPEED. > As Thomas pointed, running thorough tests by default would take more time/more disk space, and I'm not sure we want to change what is done in our CI or for people by default. This change is just supposed to be convenience for people using meson test directly, without impacting "make check*" users. So when you want to run a single test, it's visible from "meson test -C build --list", by default. > Paolo > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org > <mailto:pierrick.bouvier@linaro.org>> > --- > meson.build | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/meson.build b/meson.build > index f8bf6e7bb66..57ff3f722d8 100644 > --- a/meson.build > +++ b/meson.build > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > is_default: true, > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > env: ['RUST_BACKTRACE=1']) > add_test_setup('slow', exclude_suites: ['thorough'], > env: ['G_TEST_SLOW=1', 'SPEED=slow', > 'RUST_BACKTRACE=1']) > add_test_setup('thorough', > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', > 'RUST_BACKTRACE=1']) > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', > 'RUST_BACKTRACE=1'], > + is_default: true) > > meson.add_postconf_script(find_program('scripts/symlink-install- > tree.py')) > > -- > 2.47.2 >
On 5/5/25 3:32 AM, Thomas Huth wrote: > On 03/05/2025 22.18, Pierrick Bouvier wrote: >> Allows all tests to be visible by default when using meson test >> directly. >> >> This has no impact on make check-* commands, which use SPEED=quick by >> default (see scripts/mtest2make.py). >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> meson.build | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/meson.build b/meson.build >> index f8bf6e7bb66..57ff3f722d8 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', >> >> meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) >> >> -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, >> +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], >> env: ['RUST_BACKTRACE=1']) >> add_test_setup('slow', exclude_suites: ['thorough'], >> env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) >> add_test_setup('thorough', >> - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) >> + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], >> + is_default: true) > > I'd rather not make it the default: The thorough functional tests download a > lot of assets from the internet, so if someone just runs "meson test" > without further parameters, I think we should not trigger these downloads in > that case. > $ cat tests/Makefile.include ... check-functional: @$(NINJA) precache-functional @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func ... What's the rationale to run check-func with SPEED=thorough and QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick tests by default? Either we should remove SPEED=thorough, or download all tests by default. > Thomas >
On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: > On 5/5/25 3:32 AM, Thomas Huth wrote: > > On 03/05/2025 22.18, Pierrick Bouvier wrote: > > > Allows all tests to be visible by default when using meson test > > > directly. > > > > > > This has no impact on make check-* commands, which use SPEED=quick by > > > default (see scripts/mtest2make.py). > > > > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > > --- > > > meson.build | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/meson.build b/meson.build > > > index f8bf6e7bb66..57ff3f722d8 100644 > > > --- a/meson.build > > > +++ b/meson.build > > > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, > > > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > > > env: ['RUST_BACKTRACE=1']) > > > add_test_setup('slow', exclude_suites: ['thorough'], > > > env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) > > > add_test_setup('thorough', > > > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) > > > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], > > > + is_default: true) > > > > I'd rather not make it the default: The thorough functional tests download a > > lot of assets from the internet, so if someone just runs "meson test" > > without further parameters, I think we should not trigger these downloads in > > that case. > > > > $ cat tests/Makefile.include > ... > check-functional: > @$(NINJA) precache-functional > @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func > ... > > What's the rationale to run check-func with SPEED=thorough and > QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick > tests by default? Are you sure about that ? The precache logic uses custom_target() in meson and so does not filters on "suites" used to define quick vs thorough tests. IOW, precache should always be downloading all assets. > Either we should remove SPEED=thorough, or download all tests by default. > > > Thomas > > > With regards, Daniel
On Sat, May 03, 2025 at 01:18:06PM -0700, Pierrick Bouvier wrote: > Allows all tests to be visible by default when using meson test > directly. > > This has no impact on make check-* commands, which use SPEED=quick by > default (see scripts/mtest2make.py). IMHO it would be conceptually confusing if we cause 'make check' to be running different stuff from 'meson test'. As long as we keep a makefile wrapper around running tests, I think we should keep them matching what they do by default. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > meson.build | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/meson.build b/meson.build > index f8bf6e7bb66..57ff3f722d8 100644 > --- a/meson.build > +++ b/meson.build > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > env: ['RUST_BACKTRACE=1']) > add_test_setup('slow', exclude_suites: ['thorough'], > env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) > add_test_setup('thorough', > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], > + is_default: true) > > meson.add_postconf_script(find_program('scripts/symlink-install-tree.py')) > > -- > 2.47.2 > With regards, Daniel
On 06/05/2025 11.36, Daniel P. Berrangé wrote: > On Sat, May 03, 2025 at 01:18:06PM -0700, Pierrick Bouvier wrote: >> Allows all tests to be visible by default when using meson test >> directly. >> >> This has no impact on make check-* commands, which use SPEED=quick by >> default (see scripts/mtest2make.py). > > IMHO it would be conceptually confusing if we cause 'make check' to > be running different stuff from 'meson test'. As long as we keep a > makefile wrapper around running tests, I think we should keep them > matching what they do by default. But what we should maybe do instead: Properly document in docs/devel/testing/functional.rst how to get the list of all tests and how to run a single test with the "meson test" way... I could do it later when time permits (likely not this week), but if anybody else wants to have a try, that would be welcome! Thomas
On 5/6/25 2:33 AM, Daniel P. Berrangé wrote: > On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: >> On 5/5/25 3:32 AM, Thomas Huth wrote: >>> On 03/05/2025 22.18, Pierrick Bouvier wrote: >>>> Allows all tests to be visible by default when using meson test >>>> directly. >>>> >>>> This has no impact on make check-* commands, which use SPEED=quick by >>>> default (see scripts/mtest2make.py). >>>> >>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> --- >>>> meson.build | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/meson.build b/meson.build >>>> index f8bf6e7bb66..57ff3f722d8 100644 >>>> --- a/meson.build >>>> +++ b/meson.build >>>> @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', >>>> meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) >>>> -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, >>>> +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], >>>> env: ['RUST_BACKTRACE=1']) >>>> add_test_setup('slow', exclude_suites: ['thorough'], >>>> env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) >>>> add_test_setup('thorough', >>>> - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) >>>> + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], >>>> + is_default: true) >>> >>> I'd rather not make it the default: The thorough functional tests download a >>> lot of assets from the internet, so if someone just runs "meson test" >>> without further parameters, I think we should not trigger these downloads in >>> that case. >>> >> >> $ cat tests/Makefile.include >> ... >> check-functional: >> @$(NINJA) precache-functional >> @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func >> ... >> >> What's the rationale to run check-func with SPEED=thorough and >> QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick >> tests by default? > > Are you sure about that ? The precache logic uses custom_target() in > meson and so does not filters on "suites" used to define quick vs > thorough tests. > My bad, I missed the "foreach speed : ['quick', 'thorough']" wrapping all this in tests/functional/meson.build. > IOW, precache should always be downloading all assets. > Then I don't understand the previous argument from Thomas to not make thorough the default: "The thorough functional tests download a lot of assets from the internet, so if someone just runs "meson test" without further parameters, I think we should not trigger these downloads in that case". It's what precache-functional is doing. >> Either we should remove SPEED=thorough, or download all tests by default. >> >>> Thomas >>> >> > > With regards, > Daniel
On Tue, May 06, 2025 at 08:31:07AM -0700, Pierrick Bouvier wrote: > On 5/6/25 2:33 AM, Daniel P. Berrangé wrote: > > On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: > > > On 5/5/25 3:32 AM, Thomas Huth wrote: > > > > On 03/05/2025 22.18, Pierrick Bouvier wrote: > > > > > Allows all tests to be visible by default when using meson test > > > > > directly. > > > > > > > > > > This has no impact on make check-* commands, which use SPEED=quick by > > > > > default (see scripts/mtest2make.py). > > > > > > > > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > > > > --- > > > > > meson.build | 5 +++-- > > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/meson.build b/meson.build > > > > > index f8bf6e7bb66..57ff3f722d8 100644 > > > > > --- a/meson.build > > > > > +++ b/meson.build > > > > > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > > > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > > > > -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, > > > > > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > > > > > env: ['RUST_BACKTRACE=1']) > > > > > add_test_setup('slow', exclude_suites: ['thorough'], > > > > > env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) > > > > > add_test_setup('thorough', > > > > > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) > > > > > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], > > > > > + is_default: true) > > > > > > > > I'd rather not make it the default: The thorough functional tests download a > > > > lot of assets from the internet, so if someone just runs "meson test" > > > > without further parameters, I think we should not trigger these downloads in > > > > that case. > > > > > > > > > > $ cat tests/Makefile.include > > > ... > > > check-functional: > > > @$(NINJA) precache-functional > > > @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func > > > ... > > > > > > What's the rationale to run check-func with SPEED=thorough and > > > QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick > > > tests by default? > > > > Are you sure about that ? The precache logic uses custom_target() in > > meson and so does not filters on "suites" used to define quick vs > > thorough tests. > > > > My bad, I missed the "foreach speed : ['quick', 'thorough']" wrapping all > this in tests/functional/meson.build. > > > IOW, precache should always be downloading all assets. > > > > Then I don't understand the previous argument from Thomas to not make > thorough the default: "The thorough functional tests download a > lot of assets from the internet, so if someone just runs "meson test" > without further parameters, I think we should not trigger these downloads in > that case". It's what precache-functional is doing. I guess even without the download overhead, the tests using real VMs are going to be slower overall as they're doing more work to bring up a full OS install ? I have not quantified that difference though, and whether it is really due to the type of images being used, or whether it is just a perception from the fact that we'd be simply running more tests overall. With regards, Daniel
On 06/05/2025 17.31, Pierrick Bouvier wrote: > On 5/6/25 2:33 AM, Daniel P. Berrangé wrote: >> On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: >>> On 5/5/25 3:32 AM, Thomas Huth wrote: >>>> On 03/05/2025 22.18, Pierrick Bouvier wrote: >>>>> Allows all tests to be visible by default when using meson test >>>>> directly. >>>>> >>>>> This has no impact on make check-* commands, which use SPEED=quick by >>>>> default (see scripts/mtest2make.py). >>>>> >>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>>> --- >>>>> meson.build | 5 +++-- >>>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/meson.build b/meson.build >>>>> index f8bf6e7bb66..57ff3f722d8 100644 >>>>> --- a/meson.build >>>>> +++ b/meson.build >>>>> @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', >>>>> meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) >>>>> -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], >>>>> is_default: true, >>>>> +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], >>>>> env: ['RUST_BACKTRACE=1']) >>>>> add_test_setup('slow', exclude_suites: ['thorough'], >>>>> env: ['G_TEST_SLOW=1', 'SPEED=slow', >>>>> 'RUST_BACKTRACE=1']) >>>>> add_test_setup('thorough', >>>>> - env: ['G_TEST_SLOW=1', 'SPEED=thorough', >>>>> 'RUST_BACKTRACE=1']) >>>>> + env: ['G_TEST_SLOW=1', 'SPEED=thorough', >>>>> 'RUST_BACKTRACE=1'], >>>>> + is_default: true) >>>> >>>> I'd rather not make it the default: The thorough functional tests >>>> download a >>>> lot of assets from the internet, so if someone just runs "meson test" >>>> without further parameters, I think we should not trigger these >>>> downloads in >>>> that case. >>>> >>> >>> $ cat tests/Makefile.include >>> ... >>> check-functional: >>> @$(NINJA) precache-functional >>> @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func >>> ... >>> >>> What's the rationale to run check-func with SPEED=thorough and >>> QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick >>> tests by default? >> >> Are you sure about that ? The precache logic uses custom_target() in >> meson and so does not filters on "suites" used to define quick vs >> thorough tests. >> > > My bad, I missed the "foreach speed : ['quick', 'thorough']" wrapping all > this in tests/functional/meson.build. > >> IOW, precache should always be downloading all assets. >> > > Then I don't understand the previous argument from Thomas to not make > thorough the default: "The thorough functional tests download a > lot of assets from the internet, so if someone just runs "meson test" > without further parameters, I think we should not trigger these downloads in > that case". It's what precache-functional is doing. precache-functional is *only* called when you run "make check-functional", i.e. when you know that you want to run the functional tests that might download assets from the internet. It's not called when you run the normal "make check". I think "meson test" should by default also not download any assets from the internet (e.g. in case someone is still on a metered internet connection or tries to do air-gapped builds) - we should only download if people really really want to run the functional tests consciously. Thomas
On Tue, May 06, 2025 at 07:12:53PM +0200, Thomas Huth wrote: > On 06/05/2025 17.31, Pierrick Bouvier wrote: > > On 5/6/25 2:33 AM, Daniel P. Berrangé wrote: > > > On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: > > > > On 5/5/25 3:32 AM, Thomas Huth wrote: > > > > > On 03/05/2025 22.18, Pierrick Bouvier wrote: > > > > > > Allows all tests to be visible by default when using meson test > > > > > > directly. > > > > > > > > > > > > This has no impact on make check-* commands, which use SPEED=quick by > > > > > > default (see scripts/mtest2make.py). > > > > > > > > > > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > > > > > --- > > > > > > meson.build | 5 +++-- > > > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/meson.build b/meson.build > > > > > > index f8bf6e7bb66..57ff3f722d8 100644 > > > > > > --- a/meson.build > > > > > > +++ b/meson.build > > > > > > @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', > > > > > > meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) > > > > > > -add_test_setup('quick', exclude_suites: ['slow', > > > > > > 'thorough'], is_default: true, > > > > > > +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], > > > > > > env: ['RUST_BACKTRACE=1']) > > > > > > add_test_setup('slow', exclude_suites: ['thorough'], > > > > > > env: ['G_TEST_SLOW=1', 'SPEED=slow', > > > > > > 'RUST_BACKTRACE=1']) > > > > > > add_test_setup('thorough', > > > > > > - env: ['G_TEST_SLOW=1', 'SPEED=thorough', > > > > > > 'RUST_BACKTRACE=1']) > > > > > > + env: ['G_TEST_SLOW=1', 'SPEED=thorough', > > > > > > 'RUST_BACKTRACE=1'], > > > > > > + is_default: true) > > > > > > > > > > I'd rather not make it the default: The thorough functional > > > > > tests download a > > > > > lot of assets from the internet, so if someone just runs "meson test" > > > > > without further parameters, I think we should not trigger > > > > > these downloads in > > > > > that case. > > > > > > > > > > > > > $ cat tests/Makefile.include > > > > ... > > > > check-functional: > > > > @$(NINJA) precache-functional > > > > @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func > > > > ... > > > > > > > > What's the rationale to run check-func with SPEED=thorough and > > > > QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick > > > > tests by default? > > > > > > Are you sure about that ? The precache logic uses custom_target() in > > > meson and so does not filters on "suites" used to define quick vs > > > thorough tests. > > > > > > > My bad, I missed the "foreach speed : ['quick', 'thorough']" wrapping > > all this in tests/functional/meson.build. > > > > > IOW, precache should always be downloading all assets. > > > > > > > Then I don't understand the previous argument from Thomas to not make > > thorough the default: "The thorough functional tests download a > > lot of assets from the internet, so if someone just runs "meson test" > > without further parameters, I think we should not trigger these > > downloads in that case". It's what precache-functional is doing. > > precache-functional is *only* called when you run "make check-functional", > i.e. when you know that you want to run the functional tests that might > download assets from the internet. It's not called when you run the normal > "make check". Are you sure ? If that's the case it was *not* my intention when i added precaching - I thought that "make check" would call "make check-functional" and thus trigger precaching ? With regards, Daniel
On 07/05/2025 09.14, Daniel P. Berrangé wrote: > On Tue, May 06, 2025 at 07:12:53PM +0200, Thomas Huth wrote: >> On 06/05/2025 17.31, Pierrick Bouvier wrote: >>> On 5/6/25 2:33 AM, Daniel P. Berrangé wrote: >>>> On Mon, May 05, 2025 at 10:46:52AM -0700, Pierrick Bouvier wrote: >>>>> On 5/5/25 3:32 AM, Thomas Huth wrote: >>>>>> On 03/05/2025 22.18, Pierrick Bouvier wrote: >>>>>>> Allows all tests to be visible by default when using meson test >>>>>>> directly. >>>>>>> >>>>>>> This has no impact on make check-* commands, which use SPEED=quick by >>>>>>> default (see scripts/mtest2make.py). >>>>>>> >>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>>>>> --- >>>>>>> meson.build | 5 +++-- >>>>>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/meson.build b/meson.build >>>>>>> index f8bf6e7bb66..57ff3f722d8 100644 >>>>>>> --- a/meson.build >>>>>>> +++ b/meson.build >>>>>>> @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', >>>>>>> meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) >>>>>>> -add_test_setup('quick', exclude_suites: ['slow', >>>>>>> 'thorough'], is_default: true, >>>>>>> +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], >>>>>>> env: ['RUST_BACKTRACE=1']) >>>>>>> add_test_setup('slow', exclude_suites: ['thorough'], >>>>>>> env: ['G_TEST_SLOW=1', 'SPEED=slow', >>>>>>> 'RUST_BACKTRACE=1']) >>>>>>> add_test_setup('thorough', >>>>>>> - env: ['G_TEST_SLOW=1', 'SPEED=thorough', >>>>>>> 'RUST_BACKTRACE=1']) >>>>>>> + env: ['G_TEST_SLOW=1', 'SPEED=thorough', >>>>>>> 'RUST_BACKTRACE=1'], >>>>>>> + is_default: true) >>>>>> >>>>>> I'd rather not make it the default: The thorough functional >>>>>> tests download a >>>>>> lot of assets from the internet, so if someone just runs "meson test" >>>>>> without further parameters, I think we should not trigger >>>>>> these downloads in >>>>>> that case. >>>>>> >>>>> >>>>> $ cat tests/Makefile.include >>>>> ... >>>>> check-functional: >>>>> @$(NINJA) precache-functional >>>>> @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func >>>>> ... >>>>> >>>>> What's the rationale to run check-func with SPEED=thorough and >>>>> QEMU_TEST_NO_DOWNLOAD=1 with precache-functional having cached only quick >>>>> tests by default? >>>> >>>> Are you sure about that ? The precache logic uses custom_target() in >>>> meson and so does not filters on "suites" used to define quick vs >>>> thorough tests. >>>> >>> >>> My bad, I missed the "foreach speed : ['quick', 'thorough']" wrapping >>> all this in tests/functional/meson.build. >>> >>>> IOW, precache should always be downloading all assets. >>>> >>> >>> Then I don't understand the previous argument from Thomas to not make >>> thorough the default: "The thorough functional tests download a >>> lot of assets from the internet, so if someone just runs "meson test" >>> without further parameters, I think we should not trigger these >>> downloads in that case". It's what precache-functional is doing. >> >> precache-functional is *only* called when you run "make check-functional", >> i.e. when you know that you want to run the functional tests that might >> download assets from the internet. It's not called when you run the normal >> "make check". > > Are you sure ? If that's the case it was *not* my intention when i > added precaching - I thought that "make check" would call > "make check-functional" and thus trigger precaching ? "check-functional" is not part of the normal "check" target - on purpose since we don't want to trigger downloads by surprise when people just run "make check". That's also why we have separate "functional" CI jobs in the gitlab CI, since otherwise this would be handled by the normal "check" jobs already. Thomas
On 5/7/25 12:39 AM, Thomas Huth wrote: >>>> Then I don't understand the previous argument from Thomas to not make >>>> thorough the default: "The thorough functional tests download a >>>> lot of assets from the internet, so if someone just runs "meson test" >>>> without further parameters, I think we should not trigger these >>>> downloads in that case". It's what precache-functional is doing. >>> >>> precache-functional is *only* called when you run "make check-functional", >>> i.e. when you know that you want to run the functional tests that might >>> download assets from the internet. It's not called when you run the normal >>> "make check". >> >> Are you sure ? If that's the case it was *not* my intention when i >> added precaching - I thought that "make check" would call >> "make check-functional" and thus trigger precaching ? > > "check-functional" is not part of the normal "check" target - on purpose > since we don't want to trigger downloads by surprise when people just run > "make check". That's also why we have separate "functional" CI jobs in the > gitlab CI, since otherwise this would be handled by the normal "check" jobs > already. > `make check` calls build/pyvenv/bin/meson test --no-rebuild -t 1, which triggers func-quick by default, triggering associated downloads, since QEMU_TEST_NO_DOWNLOAD=1 is not set for this target, except if I missed another hidden hack somewhere. `meson test --no-suite func-quick --no-suite func-thorough` should be what make check should do instead, or we need another "setup", to exclude functional-quick from quick default. It seems that we just have some confusion steaming from our own "makefile abstraction layer", when we could simply use builtin meson test command for that, which works for all workflows (single vs multiple tests). It reinforces my opinion that setups are awkward and that's better to use meson test directly with --setup thorough being the default, for better or worse. In case a developer wants a specific suite, it's also easy to select it with '--suite', and list them with '--list'. That said, I'm dropping this series, and won't pursue asking to change the default, since this opinion is not the established consensus. > Thomas > Regards, Pierrick
On 07/05/2025 20.45, Pierrick Bouvier wrote: > On 5/7/25 12:39 AM, Thomas Huth wrote: >>>>> Then I don't understand the previous argument from Thomas to not make >>>>> thorough the default: "The thorough functional tests download a >>>>> lot of assets from the internet, so if someone just runs "meson test" >>>>> without further parameters, I think we should not trigger these >>>>> downloads in that case". It's what precache-functional is doing. >>>> >>>> precache-functional is *only* called when you run "make check-functional", >>>> i.e. when you know that you want to run the functional tests that might >>>> download assets from the internet. It's not called when you run the normal >>>> "make check". >>> >>> Are you sure ? If that's the case it was *not* my intention when i >>> added precaching - I thought that "make check" would call >>> "make check-functional" and thus trigger precaching ? >> >> "check-functional" is not part of the normal "check" target - on purpose >> since we don't want to trigger downloads by surprise when people just run >> "make check". That's also why we have separate "functional" CI jobs in the >> gitlab CI, since otherwise this would be handled by the normal "check" jobs >> already. >> > > `make check` calls build/pyvenv/bin/meson test --no-rebuild -t 1, which > triggers func-quick by default, triggering associated downloads, since > QEMU_TEST_NO_DOWNLOAD=1 is not set for this target, except if I missed > another hidden hack somewhere. You missed the fact that the "quick" functional tests do not download any assets :-) I know it's confusing since the name of the suites rather indicate something about the runtime of the tests and not about whether they trigger a download or not, but the suite names are so deeply glued into the "mtest2make" logic that I was not able to come up with a better solution. Maybe Paolo or some other Meson expert could clean that up, but for the time being, for the functional test we have: - quick tests that can always run (also run during "make check") - thorough tests that download assets from the internet (only run during "make check-functional") I tried to document it in docs/devel/testing/functional.rst in the "Asset handling" section already, please have a look whether that's sufficient, or whether you have some ideas how to improve the situation. Thomas
On 5/8/25 3:05 AM, Thomas Huth wrote: > On 07/05/2025 20.45, Pierrick Bouvier wrote: >> On 5/7/25 12:39 AM, Thomas Huth wrote: >>>>>> Then I don't understand the previous argument from Thomas to not make >>>>>> thorough the default: "The thorough functional tests download a >>>>>> lot of assets from the internet, so if someone just runs "meson test" >>>>>> without further parameters, I think we should not trigger these >>>>>> downloads in that case". It's what precache-functional is doing. >>>>> >>>>> precache-functional is *only* called when you run "make check-functional", >>>>> i.e. when you know that you want to run the functional tests that might >>>>> download assets from the internet. It's not called when you run the normal >>>>> "make check". >>>> >>>> Are you sure ? If that's the case it was *not* my intention when i >>>> added precaching - I thought that "make check" would call >>>> "make check-functional" and thus trigger precaching ? >>> >>> "check-functional" is not part of the normal "check" target - on purpose >>> since we don't want to trigger downloads by surprise when people just run >>> "make check". That's also why we have separate "functional" CI jobs in the >>> gitlab CI, since otherwise this would be handled by the normal "check" jobs >>> already. >>> >> >> `make check` calls build/pyvenv/bin/meson test --no-rebuild -t 1, which >> triggers func-quick by default, triggering associated downloads, since >> QEMU_TEST_NO_DOWNLOAD=1 is not set for this target, except if I missed >> another hidden hack somewhere. > > You missed the fact that the "quick" functional tests do not download any > assets :-) > Hum...... (fact push -f on my brain).... ok. > I know it's confusing since the name of the suites rather indicate something > about the runtime of the tests and not about whether they trigger a download > or not, but the suite names are so deeply glued into the "mtest2make" logic > that I was not able to come up with a better solution. Maybe Paolo or some > other Meson expert could clean that up, but for the time being, for the > functional test we have: > I understand there is sometimes a rationale reason behind things, and most of the time, historical reasons. > - quick tests that can always run (also run during "make check") > > - thorough tests that download assets from the internet (only run during > "make check-functional") > > I tried to document it in docs/devel/testing/functional.rst in the "Asset > handling" section already, please have a look whether that's sufficient, or > whether you have some ideas how to improve the situation. > Indeed, and I remember reading this when I started working on QEMU, but not really after spending time to craft the "right" command and keep it in my aliases. In general, having documentation is great, but I would still favor intuitive commands and defaults over a lengthy explaination or this email thread, but as "good defaults" and "intuitive command" is definitely a personal judgment, and not something objective, I'll simply accept the current situation and move on. > Thomas > Thanks for the insights, Pierrick
diff --git a/meson.build b/meson.build index f8bf6e7bb66..57ff3f722d8 100644 --- a/meson.build +++ b/meson.build @@ -5,12 +5,13 @@ project('qemu', ['c'], meson_version: '>=1.5.0', meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], env: ['RUST_BACKTRACE=1']) add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) add_test_setup('thorough', - env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1'], + is_default: true) meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
Allows all tests to be visible by default when using meson test directly. This has no impact on make check-* commands, which use SPEED=quick by default (see scripts/mtest2make.py). Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- meson.build | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)