Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
Le 23/02/2019 à 18:32, gregor herrmann a écrit : > On Sat, 16 Feb 2019 19:52:06 +0200, Niko Tyni wrote: > >> I agree with the intention here. The only non-superficial test of the >> current ones is indeed the 'smoke' test. > > Cool, thanks. > >> However, I have an "architectural" concern. > > […] > > Makes sense. Maybe material for the next sprint? > >> Please don't consider this a blocker or veto for the proposed >> implementation. It's a clear improvement and does not make >> it any harder to simplify the architecture later. > > Ok, so I guess we can merge this change? > Xavier, I seem to remember that this has to go into autodep8 first or > something like that? Hello, Right, pushed (https://salsa.debian.org/ci-team/autodep8/merge_requests/13). We have to wait for autodep8 release with with MR merged before accepting pkg-perl-tools MR, else autopkgtest will fail. Cheers, Xavier
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
On Sat, 16 Feb 2019 19:52:06 +0200, Niko Tyni wrote: > I agree with the intention here. The only non-superficial test of the > current ones is indeed the 'smoke' test. Cool, thanks. > However, I have an "architectural" concern. […] Makes sense. Maybe material for the next sprint? > Please don't consider this a blocker or veto for the proposed > implementation. It's a clear improvement and does not make > it any harder to simplify the architecture later. Ok, so I guess we can merge this change? Xavier, I seem to remember that this has to go into autodep8 first or something like that? Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Sophie Hunger: The Tourist signature.asc Description: Digital Signature
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
On Fri, Feb 15, 2019 at 08:55:16PM +0100, gregor herrmann wrote: > > > - As for the implementation in [0]: > > > not sure if the "exit 0" in smoke is correct > > This still confuses me. > Shouldn't it "exit $?" or just nothing (line 174)? It's a "set -e" script so a failure from test.pl should stop processing before the 'exit 0'. But yeah, the flow could be a bit clearer. > > > - What about the skipped tests within use.t and syntax.t? Should they > > > or some of them also exit 77? > > runner do it for them. > > Well, only partially. First of all, runners can run more than one > test in a subdirectory (even if we currently only have 3 files in 4 > subdirectories, with 3 times 1 and 1 time zero), second, there are > several places in syntax.t and use.t were all or parts of the tests > are skipped. -- But: > > > I didn't modify them if all is skipped as it has > > no effect on a test marked as "superficial": 0 or 77 gives the same > > result: no benefit, no penalty > > … this "no benefit, no penalty" makes it indeed kind of moot :) Given we need to mark the superficial tests as skippable (due to the runner change), it would seem cleaner to me to make them signal properly also the other cases of skipping. But I can see it doesn't matter that much. > (Random note, so we don't forget it: There are a few adjusted copies > of the autodep8 file in various packages as debian/tests/control > which should also be adjusted, at least in git for the next upload.) AIUI these changes only affect packages without proper 'smoke' tests. When 'smoke' is working correctly, neither the 'superficial' nor the 'skippable' part really matters. So adjusting those individual copies doesn't seem much of a priority unless there are some where 'smoke' is skipped (or left out). -- Niko
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
On Wed, Feb 13, 2019 at 09:23:00PM +0100, Xavier Guimard wrote: > Package: pkg-perl-autopkgtest > Version: 0.50 > Severity: wishlist > - tests skipped should return a 77 exit code and all tests marked as >"Restrictions: skippable". It avoids to consider that a test succeeds >if maintainer skipped it, but needs a merge request to autodep8. See > > https://salsa.debian.org/ci-team/autodep8/blob/master/support/nodejs/generate >(changed by MR !11) Makes sense to me. > - runtime-deps* tests should be tagged as "Restrictions: superficial" >since these tests don't really test package features but just Perl >syntax > > Then with this 2 changes, if "build-deps.d" is skipped, success won't > give the benefit of 3-days-reduce. I agree with the intention here. The only non-superficial test of the current ones is indeed the 'smoke' test. However, I have an "architectural" concern. The proposed implementation overloads the 'runtime-deps' vs 'build-deps' separation. Theoretically, we might come up with a 'runtime-deps' test that is not superficial even though all of them currently are, or a 'build-deps' test that is superficial unlike the current ones. The separation precedes autodep8 and was designed to make it possible to update the set of standard pkg-perl autopkgtests without having to update every package separately. This was later solved in a more centralized way by autodep8. I think we should probably get rid of this now unnecessary layer of indirection and just list smoke, use.t and syntax.t in autodep8, with the proper restrictions. This seems buster+1 material. Please don't consider this a blocker or veto for the proposed implementation. It's a clear improvement and does not make it any harder to simplify the architecture later. Thanks for working on this, -- Niko
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
On Fri, 15 Feb 2019 18:29:14 +0100, Xavier wrote: > > For the skippable part: > > - If I understand this correctly (from your text above and the spec > > [1]) then a skipped syntax.t and use.t would also lead to losing > > the benefit of faster migration? Do we want this? > The benefit will be lost only if smoke test is skipped. I think it's a > good thing (other tests are "superficial" <=> no benefit). Today if this > test is skipped, it is considered by autopkgtest as "success" Ah, I see. Ok, a lost benefit only for the skipped smoke tests probably makes sense. > > - As for the implementation in [0]: > > not sure if the "exit 0" in smoke is correct This still confuses me. Shouldn't it "exit $?" or just nothing (line 174)? > > - What about the skipped tests within use.t and syntax.t? Should they > > or some of them also exit 77? > runner do it for them. Well, only partially. First of all, runners can run more than one test in a subdirectory (even if we currently only have 3 files in 4 subdirectories, with 3 times 1 and 1 time zero), second, there are several places in syntax.t and use.t were all or parts of the tests are skipped. -- But: > I didn't modify them if all is skipped as it has > no effect on a test marked as "superficial": 0 or 77 gives the same > result: no benefit, no penalty … this "no benefit, no penalty" makes it indeed kind of moot :) > > In general I still don't have the full picture of what benefits and > > penalties for testing migration will result from which combination of > > the changes under which circumstances. > > The only effect of this is that if smoke test is skipped, there is no > benefit. And I think it's more clear to have the real result: > > # EXAMPLE 1, SKIP use.t => benefit OK > autopkgtest [18:23:17]: summary > command1: PASS > command2: FAIL exit 77 (marked as skippable) # I don't remember the > # exact message > command3: PASS (superficial) > > # EXAMPLE 2, SKIP smoke => no benefit > command1: FAIL exit 77 (marked as skippable) > command2: PASS (superficial) > command3: PASS (superficial) > > # EXAMPLE 3, real failure in smoke => penalty > command1: FAIL > command2: PASS (superficial) > command3: PASS (superficial) > > # EXAMPLE 4, real failure in use.t => penalty > command1: PASS > command2: FAIL > command3: PASS (superficial) Thanks alot for those examples, they make it indeed easier for me to understand the effects! So, hm, yeah, I guess that all makes sense … I hope someone else also has some minutes to think it through :) (Random note, so we don't forget it: There are a few adjusted copies of the autodep8 file in various packages as debian/tests/control which should also be adjusted, at least in git for the next upload.) Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Josh With: Milk cow blues signature.asc Description: Digital Signature
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
Le 15/02/2019 à 18:29, Xavier a écrit : > Le 15/02/2019 à 17:57, gregor herrmann a écrit : >> On Wed, 13 Feb 2019 21:23:00 +0100, Xavier Guimard wrote: >> >>> Some suggestions for pkg-js-autopkgtest based on pkg-js-autopkgtest >>> discussion with autodep8 maintainers: >>> - tests skipped should return a 77 exit code and all tests marked as >>>"Restrictions: skippable". It avoids to consider that a test succeeds >>>if maintainer skipped it, but needs a merge request to autodep8. See >>> >>> https://salsa.debian.org/ci-team/autodep8/blob/master/support/nodejs/generate >>>(changed by MR !11) >>> - runtime-deps* tests should be tagged as "Restrictions: superficial" >>>since these tests don't really test package features but just Perl >>>syntax >>> >>> Then with this 2 changes, if "build-deps.d" is skipped, success won't >>> give the benefit of 3-days-reduce. >> >> Thanks for your work and the pull request [0]! >> >> Some thoughts and more questions: >> >> For the skippable part: >> - If I understand this correctly (from your text above and the spec >> [1]) then a skipped syntax.t and use.t would also lead to losing >> the benefit of faster migration? Do we want this? > > The benefit will be lost only if smoke test is skipped. I think it's a > good thing (other tests are "superficial" <=> no benefit). Today if this > test is skipped, it is considered by autopkgtest as "success" > >> Or does it just have no influence? > > skippable has no influence if test succeed, but don't consider that test > has failed if a 77 code is returned > >> - As for the implementation in [0]: >> not sure if the "exit 0" in smoke is correct >> - What about the skipped tests within use.t and syntax.t? Should they >> or some of them also exit 77? > > runner do it for them. I didn't modify them if all is skipped as it has > no effect on a test marked as "superficial": 0 or 77 gives the same > result: no benefit, no penalty > >> For the superficial part: >> Hm, yeah, use.t and syntax.t don't test that everything in the >> package is fully functional; still, this "superficial" feels a bit >> weird. But probably it's correct according to [1]. >> >> In general I still don't have the full picture of what benefits and >> penalties for testing migration will result from which combination of >> the changes under which circumstances. > > The only effect of this is that if smoke test is skipped, there is no > benefit. And I think it's more clear to have the real result: I found the exact message, it is very clear: # EXAMPLE 1, SKIP use.t => benefit OK autopkgtest [18:23:17]: summary command1: PASS command2: SKIP exit status 77 and marked as skippable command3: PASS (superficial) # EXAMPLE 2, SKIP smoke => no benefit command1: SKIP exit status 77 and marked as skippable command2: PASS (superficial) command3: PASS (superficial) # EXAMPLE 3, real failure in smoke => penalty command1: FAIL command2: PASS (superficial) command3: PASS (superficial) # EXAMPLE 4, real failure in use.t => penalty command1: PASS command2: FAIL command3: PASS (superficial) Cheers, Xavier
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
Le 15/02/2019 à 17:57, gregor herrmann a écrit : > On Wed, 13 Feb 2019 21:23:00 +0100, Xavier Guimard wrote: > >> Some suggestions for pkg-js-autopkgtest based on pkg-js-autopkgtest >> discussion with autodep8 maintainers: >> - tests skipped should return a 77 exit code and all tests marked as >>"Restrictions: skippable". It avoids to consider that a test succeeds >>if maintainer skipped it, but needs a merge request to autodep8. See >> >> https://salsa.debian.org/ci-team/autodep8/blob/master/support/nodejs/generate >>(changed by MR !11) >> - runtime-deps* tests should be tagged as "Restrictions: superficial" >>since these tests don't really test package features but just Perl >>syntax >> >> Then with this 2 changes, if "build-deps.d" is skipped, success won't >> give the benefit of 3-days-reduce. > > Thanks for your work and the pull request [0]! > > Some thoughts and more questions: > > For the skippable part: > - If I understand this correctly (from your text above and the spec > [1]) then a skipped syntax.t and use.t would also lead to losing > the benefit of faster migration? Do we want this? The benefit will be lost only if smoke test is skipped. I think it's a good thing (other tests are "superficial" <=> no benefit). Today if this test is skipped, it is considered by autopkgtest as "success" > Or does it just have no influence? skippable has no influence if test succeed, but don't consider that test has failed if a 77 code is returned > - As for the implementation in [0]: > not sure if the "exit 0" in smoke is correct > - What about the skipped tests within use.t and syntax.t? Should they > or some of them also exit 77? runner do it for them. I didn't modify them if all is skipped as it has no effect on a test marked as "superficial": 0 or 77 gives the same result: no benefit, no penalty > For the superficial part: > Hm, yeah, use.t and syntax.t don't test that everything in the > package is fully functional; still, this "superficial" feels a bit > weird. But probably it's correct according to [1]. > > In general I still don't have the full picture of what benefits and > penalties for testing migration will result from which combination of > the changes under which circumstances. The only effect of this is that if smoke test is skipped, there is no benefit. And I think it's more clear to have the real result: # EXAMPLE 1, SKIP use.t => benefit OK autopkgtest [18:23:17]: summary command1: PASS command2: FAIL exit 77 (marked as skippable) # I don't remember the # exact message command3: PASS (superficial) # EXAMPLE 2, SKIP smoke => no benefit command1: FAIL exit 77 (marked as skippable) command2: PASS (superficial) command3: PASS (superficial) # EXAMPLE 3, real failure in smoke => penalty command1: FAIL command2: PASS (superficial) command3: PASS (superficial) # EXAMPLE 4, real failure in use.t => penalty command1: PASS command2: FAIL command3: PASS (superficial) Cheers, Xavier
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
On Wed, 13 Feb 2019 21:23:00 +0100, Xavier Guimard wrote: > Some suggestions for pkg-js-autopkgtest based on pkg-js-autopkgtest > discussion with autodep8 maintainers: > - tests skipped should return a 77 exit code and all tests marked as >"Restrictions: skippable". It avoids to consider that a test succeeds >if maintainer skipped it, but needs a merge request to autodep8. See > > https://salsa.debian.org/ci-team/autodep8/blob/master/support/nodejs/generate >(changed by MR !11) > - runtime-deps* tests should be tagged as "Restrictions: superficial" >since these tests don't really test package features but just Perl >syntax > > Then with this 2 changes, if "build-deps.d" is skipped, success won't > give the benefit of 3-days-reduce. Thanks for your work and the pull request [0]! Some thoughts and more questions: For the skippable part: - If I understand this correctly (from your text above and the spec [1]) then a skipped syntax.t and use.t would also lead to losing the benefit of faster migration? Do we want this? Or does it just have no influence? - As for the implementation in [0]: not sure if the "exit 0" in smoke is correct - What about the skipped tests within use.t and syntax.t? Should they or some of them also exit 77? For the superficial part: Hm, yeah, use.t and syntax.t don't test that everything in the package is fully functional; still, this "superficial" feels a bit weird. But probably it's correct according to [1]. In general I still don't have the full picture of what benefits and penalties for testing migration will result from which combination of the changes under which circumstances. Cheers, gregor [0] https://salsa.debian.org/perl-team/modules/packages/pkg-perl-tools/merge_requests/2 [1] https://salsa.debian.org/ci-team/autopkgtest/raw/master/doc/README.package-tests.rst -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Cat Power: The Greatest signature.asc Description: Digital Signature
Bug#922264: pkg-perl-autopkgtest: use "skippable" and "superficial" restrictions
Package: pkg-perl-autopkgtest Version: 0.50 Severity: wishlist Hi all, Some suggestions for pkg-js-autopkgtest based on pkg-js-autopkgtest discussion with autodep8 maintainers: - tests skipped should return a 77 exit code and all tests marked as "Restrictions: skippable". It avoids to consider that a test succeeds if maintainer skipped it, but needs a merge request to autodep8. See https://salsa.debian.org/ci-team/autodep8/blob/master/support/nodejs/generate (changed by MR !11) - runtime-deps* tests should be tagged as "Restrictions: superficial" since these tests don't really test package features but just Perl syntax Then with this 2 changes, if "build-deps.d" is skipped, success won't give the benefit of 3-days-reduce. Cheers, Xavier -- System Information: Debian Release: buster/sid APT prefers testing APT policy: (900, 'testing'), (500, 'unstable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.19.0-2-amd64 (SMP w/4 CPU cores) Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8), LANGUAGE= (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages pkg-perl-autopkgtest depends on: ii perl 5.28.1-4 pkg-perl-autopkgtest recommends no packages. pkg-perl-autopkgtest suggests no packages. -- no debconf information
