[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#901804: autopkgtest: consider using exit status 8 ("no tests found") if every test was ignored



Hi Simon,

First, thanks a lot for bringing this here and doing the great summary.

On 18-06-18 17:02, Simon McVittie wrote:
> There is a concern that packages should not be given a testing migration
> bonus if they were not actually tested. For example, all of bubblewrap's
> tests need isolation-machine, because unprivileged LXC containers don't
> give processes enough capabilities to exercise CAP_SYS_ADMIN (even though
> bwrap is setuid root so it could normally do that), but afaics it still
> gets the faster migration.
> 
> I run bubblewrap tests in a VM before each upload, so I don't want to
> remove the tests altogether; but Debian shouldn't have to take my word
> for it when I say I ran the tests :-)
> 
> The introduction of skippable and flaky test Restrictions in git master
> potentially exacerbates this, by making it more straightforward to
> represent tests that can't always give a result (but at the moment the
> state of the art is to make these trivially "pass", so providing more
> information can't possibly be a negative).
> 
> My proposal was to consider making autopkgtest treat "no test has
> succeeded or failed" the same as "there are no tests", so exit 8 if
> every test was ignored. I'm using "ignored" here to mean: skipped by
> Restrictions; skipped by 'skippable' and exiting 77; skipped by 'flaky'
> and exiting nonzero or writing non-ignored stderr; or anything similar
> that we might add in future.
> 
> That would mean the spec would be:
> 
> * exit 0: there is at least one test and all succeeded
>     * same as now
>     * overall result: strongly positive
>     * debci interpretation: pass
>     * britney interpretation: speed up migration
> 
> * exit 2: at least one test succeeded, and at least one was ignored
>   (skipped by a restriction, skippable and exited 77, or flaky and failed)
>     * almost the same as now, but require at least one success
>     * overall result: less strongly positive
>     * debci interpretation: pass (or a new pass-with-caveats?)
>     * britney interpretation: speed up migration (but less so?)

This being used for "all tests skipped" was introduced to fix bug
737242. The comment (and the code) currently explicitly reads:
if not tests:
    # if we have skipped tests, don't claim that we don't have any
    if not errorcode & 2:
        adtlog.report('*', 'SKIP no tests in this package')
        errorcode |= 8

I think that original bug is equally satisfied with exit code 8, except
the error message should not be "no tests in this package".

In the end we want migration gated by autopkgtest. I don't think it is
worth it to introduce more speeds (but that is up to the RT anyways).

> * exit 8: no test succeeded or failed (either there were no tests,
>   or there were tests but they were all ignored)
>     * same as now if there were no tests
>     * not the same as now if all tests were ignored
>     * overall result: neutral
>     * debci interpretation: a new neutral state (doesn't exist at the moment)
>     * britney interpretation: default behaviour as though there were no tests
> 
> * exit 4, 6 or 12: at least one test failed and was not ignored
>     * same as now for those exit statuses
>     * debci currently interprets status 8 like this, but doesn't see
>       packages with no tests, so in practice it doesn't happen

Have you checked that there are no packages without a test, but that
claim they have one? I think britney will schedule those, so I don't
know if this statement is true.

>     * overall result: negative (consider delaying or blocking migration)
>     * debci interpretation: fail
>     * britney interpretation: delay (or eventually block) migration
> 
> * exit with another status: testbed failure or other brokennes
>     * same as now
>     * debci interpretation: tmpfail
>     * britney interpretation: (I don't know what britney does for tmpfail)

tmpfail are currently retried by britney. Eventually I want debci to
take care of that instead. For aging, see below.

> This will need debci and possibly britney changes, to add a new neutral
> test status and report it to britney, which would interpret it as "don't
> expedite migration, but don't delay migration either".

Let me try to outline current britney behavior (from memory):

Start with default age
- unbuilt on all or (any of testable arches)
  -> wait for build
- package has build on all and a testable arch
  -> trigger tests
 * Results are not known:
   -> Add 10 days to wait for them unless urgency high or higher
 * Package introduces a regression somewhere
   -> Add 10 days
 * Package doesn't introduce a regression
  o Package has own test case that passes
   -> Reduce 3 days but not below 2 days
  o else
   -> Default age

Britneys states are
- PASS                    = neutral or speed up
- ALWAYS FAILED           = neutral
- REGRESSION              = slow down / block
- Running                 = slow down / block
- Running (always failed) = slow down / block

In conclusion, I agree.

Paul

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: