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

Re: RFR: Britney - autopkgtest integration



Paul Gevers:
> Dear release team,
> 
> Since Dec 2016⁰ I have been working together with multiple people
> (including nthykier) to prepare the integration of britney with
> autopkgtest results. We believe the work has reached a state where full
> deployment can be considered. Let me describe how it is designed to work
> and what I request from you.
> 
> 

Hi Paul,

Thanks for working on this and I am excited that we are getting ready to
deploy this. :)

> [...]
> 
> Request
> =======
> First of all, I'd like to request review of the changes I made to
> britney to facilitate the integration with autopkgtest results. My work
> lives on the autopkgtest branch of the github repository of britney². I
> would appreciate it when you could have a look at the new code and
> provide feedback.
> 

Ok, a few remarks:


AutopkgtestPolicy.__init__:
> self.pending_tests_file = os.path.join(self.options.state_dir, 'pending.json')
> self.results_cache_file = os.path.join(self.options.state_dir, 'results.cache')

The basenames ("pending.json" and "results.cache") are rather generic
and state_dir is reused between all policies.  I think we should
namespace these files somehow (E.g. either put them in a subdir or name
them "autopkgtests-<original-name>").


AutopkgtestPolicy.apply_policy_impl:
>                 # Keep track if this source package has tests of its own for the
>                 # bounty system, but only if at least one arch has something else than
>                 # running or alwaysfail
>                 if testsrc == source_name and r - {'RUNNING', 'RUNNING-ALWAYSFAIL', 'ALWAYSFAIL'}:
>                     src_has_own_test = True

How difficult would it be to make the bouncy only apply if the tests are
successful on all architectures (i.e. no "ALWAYSFAIL")?  If it is a
trivial matter, then I would prefer to only give bonus to packages that
are a good example on all architectures.


AutopkgtestPolicy.tests_for_source:
>         # Hardcode linux-meta →  linux, lxc, glibc, systemd triggers until we get a more flexible
>         # implementation: https://bugs.debian.org/779559
>         if src.startswith('linux-meta'):
>             for pkg in ['lxc', 'lxd', 'glibc', src.replace('linux-meta', 'linux'), 'systemd', 'snapd']:
>                 if pkg not in reported_pkgs:
>                     # does this have any image on this arch?
>                     for pkg_id in srcinfo.binaries:
>                         if pkg_id.architecture == arch and '-image' in pkg_id.package_name:
>                             try:
>                                 tests.append((pkg, self.britney.sources['unstable'][pkg].version))
>                             except KeyError:
>                                 try:
>                                     tests.append((pkg, sources_info[pkg].version))
>                                 except KeyError:
>                                     # package not in that series? *shrug*, then not
>                                     pass
>                             break

Is this piece still relevant?   AFAICT, #779559 is fixed in stable.  (I
admit having a feeling of deja vu about this - apologies if I asked
about this case earlier and simply forgot the answer).



Beyond that: I also committed some changes for some nits things I
discovered while reviewing the code.


> Secondly, assuming we can come to deployment of the new britney policy,
> I started a gobby document³ to draft an announcement to d-d-a. The
> document has already been commented on by nthykier and terceiro (CI),
> but I'd like it to be as good as possible.
> 

One remark I just noted:

Cite:
> There is one important note to make here on how to go about regressions in test
> cases from reverse dependencies. We really recommend *communication* between
> the maintainers of the involved packages. More information is available in the
> britney documentation [britney].

We stress "communication" in that paragraph:  What (I presume) we want
to say is that people to talk to the maintainers of the other party
(instead of the release team).

When I reread it today, I suddenly interpreted it as "We recommend that
you communicate" (with the implication that maintainers are not
communicating at all).  I know that is not what we are trying to imply,
but some people might misread it.


My "quick" fix would be removing the emphasis and inject a bit longer
explanation:
"""
There is one important note to make here on how to go about regressions
in test cases from reverse dependencies.  We recommend communication
between the maintainers of the involved packages as one party has
insight in what changed and the other party insight in what is being
tested. More information is available in the britney documentation
[britney].
"""


> [...]
> 
> Looking forward to your responses,
> Paul
> 
> [...]
> 

Once again, thanks for your hard work. :)

On another note: Would you be open for getting hint permissions for
dealing with autopkgtest policy issues (at least in the initial phase of
the deployment)?
  @RT: On a related note: Are there any concerns with me giving Paul
permission to use the autopkgtest related hints (assuming we can solve
presumably minor the technical issue of creating a hint file for a
non-RT account).

Thanks,
~Niels



Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: