Debian Bug report logs - #905885
diffoscope: allow overriding of @skip_unless_tools_exist to fail if tools are missing

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Stuart Prescott <stuart@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Sat, 11 Aug 2018 16:51:26 +1000

Package: diffoscope
Version: 99
Severity: normal

Dear Maintainers,

diffoscope recommends apktool but apktool is not currently in buster. The
test suite of diffoscope currently passes without error because the tests
that use apktool are decorated with @tool_required('apktool').

Consequently:

* the code paths that use apktool are not being tested in buster

* these tests are not actually passing but ci.debian.net is saying that
  the test suite has passed (skipping tests to hide errors isn't the
  same as a test passing)

There is clear value in being able to skip tests for the casual user on
another platform or when not interested in diffoscope's abilities in
file formats about which one does not care. For the Debian package,
however, I think every functionality of the Debian package should be
tested in autopkgtest. The current situation does not pick up that apktool
is not installable in buster, for instance, and would not pick up
something like /usr/bin/apktool being renamed.

Perhaps @tool_required could be extended with an "all tools required"
mode such that no tests are skipped or missing tools are a test failure
(or error). An environment variable would work for setting that mode
and that mode used at package build time and in autopkgtest.

BTW other latent bugs here:

* diffoscope has an unsatisfiable recommends

* diffoscope cannot be built in buster (unsatisfiable build-deps,
  this is RC)

cheers
Stuart

Message #10 received at submit@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Stuart Prescott <stuart@debian.org>, 905885@bugs.debian.org, Debian Bug Tracking System <submit@bugs.debian.org>

Subject: Re: Bug#905885: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Sat, 11 Aug 2018 08:48:59 +0100

Hi Stuart,

Braindumping here whilst I have a second...

> diffoscope recommends apktool but apktool is not currently in buster.
             ^^^^^^^^^^

Ah, this was the bit I was missing in my head. ie. not a Build-Depends :)

> Perhaps @tool_required could be extended with an "all tools required"
> mode such that no tests are skipped or missing tools are a test failure
> (or error). An environment variable would work for setting that mode
> and that mode used at package build time and in autopkgtest.

Out of curiousity, is there an environment variable that ci.debian.net
reliably exports?

(One difficulty with "just" extending @tool_required is that sometimes
we really do want to skip the tests as they require a newer/older
version of a particular tool in order that we can correctly match up
the output.)


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #20 received at 905885@bugs.debian.org (full text, mbox, reply):

From: Stuart Prescott <stuart@debian.org>

To: Chris Lamb <lamby@debian.org>

Cc: 905885@bugs.debian.org

Subject: Re: Bug#905885: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Sun, 12 Aug 2018 00:03:50 +1000

> Braindumping here whilst I have a second...
> 
> > diffoscope recommends apktool but apktool is not currently in buster.
> 
>              ^^^^^^^^^^
> 
> Ah, this was the bit I was missing in my head. ie. not a Build-Depends :)

both, actually. diffoscope FTBFS in buster because:

Build-Depends: apktool <!nocheck>, …

(which is an RC bug waiting to be filed)

*also* diffoscope recommends apktool but tests on ci.d.n do not fail (but 
should, I think)

> > Perhaps @tool_required could be extended with an "all tools required"
> > mode such that no tests are skipped or missing tools are a test failure
> > (or error). An environment variable would work for setting that mode
> > and that mode used at package build time and in autopkgtest.
> 
> Out of curiousity, is there an environment variable that ci.debian.net
> reliably exports?

You'd set that environment variable in d/rules and d/tests/* (and so could 
other distros who want to test that packages are working)

> (One difficulty with "just" extending @tool_required is that sometimes
> we really do want to skip the tests as they require a newer/older
> version of a particular tool in order that we can correctly match up
> the output.)

If the newer/older version doesn't work, surely the test should fail?

cheers
Stuart

-- 
Stuart Prescott    http://www.nanonanonano.net/   stuart@nanonanonano.net
Debian Developer   http://www.debian.org/         stuart@debian.org
GPG fingerprint    90E2 D2C1 AD14 6A1B 7EBB 891D BBC1 7EBB 1396 F2F7

Message #25 received at 905885@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Stuart Prescott <stuart@debian.org>

Cc: 905885@bugs.debian.org

Subject: Re: Bug#905885: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Sat, 11 Aug 2018 15:22:19 +0100

Hi Stuart,

> > Ah, this was the bit I was missing in my head. ie. not a Build-Depends :)
> 
> both, actually. diffoscope FTBFS in buster 
[..]

I'll leave these to Mattia has he has been playing about in this area
recently.

> You'd set that environment variable in d/rules and d/tests/* (and so could 
> other distros who want to test that packages are working)

Sure of course one would take this approach, but just out of curiousity
whether there is a 100% reliable one set by ci.debian.net?

> > (One difficulty with "just" extending @tool_required is that sometimes
> > we really do want to skip the tests as they require a newer/older
> > version of a particular tool in order that we can correctly match up
> > the output.)
> 
> If the newer/older version doesn't work, surely the test should fail?

work != differ. If it *fails*, sure (and we handle/fix those), but
ironically don't guarantee that the output of diffoscope will be reproducible
across (invented example here) ImageMagick 5.1 and 5.2.


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #28 received at 905885@bugs.debian.org (full text, mbox, reply):

From: Mattia Rizzolo <mattia@debian.org>

To: Stuart Prescott <stuart@debian.org>, 905885@bugs.debian.org

Subject: Re: Bug#905885: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Mon, 3 Sep 2018 16:42:20 +0200

[Message part 1 (text/plain, inline)]

Control: allow to override @skip_unless_tools_exist during tests and fail the test if the tool is missing

On Sun, Aug 12, 2018 at 12:03:50AM +1000, Stuart Prescott wrote:
> *also* diffoscope recommends apktool but tests on ci.d.n do not fail (but 
> should, I think)

what you say here it's half true: the autopkgtest has Depends: all the
recommends that we have.  What is happening is that ci.d.n/autopkgtest
(no clue what piece of the puzzle is responsable for this) is
downloading the test dependencies from unstable if they are not
available on testing, silently.
I believe you may want to get in touch with the CI team for this, I
don't think there is much a package can do here.

> > > Perhaps @tool_required could be extended with an "all tools required"
> > > mode such that no tests are skipped or missing tools are a test failure
> > > (or error). An environment variable would work for setting that mode
> > > and that mode used at package build time and in autopkgtest.
> > 
> > (One difficulty with "just" extending @tool_required is that sometimes
> > we really do want to skip the tests as they require a newer/older
> > version of a particular tool in order that we can correctly match up
> > the output.)

That's an interesting idea, and I'm retitling this bug to do this.
Though what we would need to twak is the @skip_unless_tools_exist
decorator we use in the tests.
Also, we have comparators (and tests) for tools that Debian doesn't
ship, so if we add a flag to make @skip_unless_tools_exist fail a test
instead of skipping it, we would also need to provide a mechanism to
"override the override" and have it really skip those :)

I'm not exactly sure how we should handle the various other
@skip_unless_tool_is_* and friends though...

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

[signature.asc (application/pgp-signature, inline)]

Message #37 received at 905885@bugs.debian.org (full text, mbox, reply):

From: Holger Levsen <holger@layer-acht.org>

To: Mattia Rizzolo <mattia@debian.org>, 905885@bugs.debian.org

Subject: Re: Bug#905885: diffoscope: skipping tests on ci.debian.net is perhaps wrong?

Date: Mon, 3 Sep 2018 16:36:07 +0000

Control: retitle -1 'allow to override @skip_unless_tools_exist during tests and fail the test if the tool is missing'


-- 
cheers,
	Holger

-------------------------------------------------------------------------------
               holger@(debian|reproducible-builds|layer-acht).org
       PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C

Message #46 received at 905885@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 905885@bugs.debian.org

Cc: Stuart Prescott <stuart@debian.org>

Subject: Re: diffoscope: allow overriding of @skip_unless_tools_exist to fail if tools are missing

Date: Sat, 15 Dec 2018 16:00:53 +0100

forwarded 905885 https://salsa.debian.org/reproducible-builds/diffoscope/issues/35
thanks

I've forwarded this upstream here:

  https://salsa.debian.org/reproducible-builds/diffoscope/issues/35


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #51 received at 905885-submitter@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 905885-submitter@bugs.debian.org

Subject: Bug #905885 in diffoscope marked as pending

Date: Tue, 23 Apr 2019 16:56:14 +0000

Control: tag -1 pending

Hello,

Bug #905885 in diffoscope reported by you has been fixed in the
Git repository and is awaiting an upload. You can see the commit
message below and you can check the diff of the fix at:

https://salsa.debian.org/reproducible-builds/diffoscope/commit/50a9572b0c49ba02c2a2e8a19cb3b763c08a5339

------------------------------------------------------------------------
Treat missing tools as failures if the DIFFOSCOPE_TESTS_FAIL_ON_MISSING_TOOLS environment variable is exported. (Closes: #905885, reproducible-builds/diffoscope#35)
------------------------------------------------------------------------

(this message was generated automatically)
-- 
Greetings

https://bugs.debian.org/905885

Message #58 received at 905885@bugs.debian.org (full text, mbox, reply):

From: "Chris Lamb" <lamby@debian.org>

To: 905885@bugs.debian.org

Cc: "Stuart Prescott" <stuart@debian.org>, "Mattia Rizzolo" <mattia@debian.org>

Subject: Re: diffoscope: allow overriding of @skip_unless_tools_exist to fail if tools are missing

Date: Tue, 23 Apr 2019 12:58:26 -0400

tags 905885 + pending
thanks

Implemented in Git, now pending upload:

  https://salsa.debian.org/reproducible-builds/diffoscope/commit/50a9572b0c49ba02c2a2e8a19cb3b763c08a5339


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #65 received at 905885-close@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 905885-close@bugs.debian.org

Subject: Bug#905885: fixed in diffoscope 114

Date: Sun, 28 Apr 2019 16:33:46 +0000

Source: diffoscope
Source-Version: 114

We believe that the bug you reported is fixed in the latest version of
diffoscope, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 905885@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Chris Lamb <lamby@debian.org> (supplier of updated diffoscope package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Sun, 28 Apr 2019 17:21:46 +0100
Source: diffoscope
Binary: diffoscope
Built-For-Profiles: nocheck
Architecture: source all
Version: 114
Distribution: experimental
Urgency: medium
Maintainer: Reproducible builds folks <reproducible-builds@lists.alioth.debian.org>
Changed-By: Chris Lamb <lamby@debian.org>
Description:
 diffoscope - in-depth comparison of files, archives, and directories
Closes: 871244 874582 888237 905885
Changes:
 diffoscope (114) experimental; urgency=medium
 .
   [ Chris Lamb ]
   * Add support for GnuPG "keybox" files. Thanks to Daniel Kahn Gillmor for the
     suggestion. (Closes: #871244, reproducible-builds/diffoscope#23)
   * Always warn if tlsh module is not available (not just if a specific
     fuzziness threshold is specified) to match the epilog of the --help output.
     This prevents missing support for file rename detection.
     (Closes: #888237, reproducible-builds/diffoscope#29)
   * Treat missing tools on Debian autopkgtests as individual test failures by
     checking whether a new DIFFOSCOPE_TESTS_FAIL_ON_MISSING_TOOLS environment
     variable is exported. (Closes: #905885, reproducible-builds/diffoscope#35)
   * Require that "-" is explicitly specified to read a single diff from
     standard input to avoid non-intuitive behaviour when diffoscope is called
     without any arguments. (Closes: reproducible-builds/diffoscope#54)
   * Make --use-dbgsym a ternary operator to make it easier to totally disable.
     Thanks to Mattia Rizzolo for the suggestion.
   * Consolidate on "e" as the aliased exception name.
 .
   [ Milena Boselli Rosa ]
   * Various fixes to the HTML markup to prevent validation warnings/errors:
     - Prevent empty values for the "name" attribute name on HTML anchor tags,
       and add an "id" to its parent "div" container.
     - Fix "table column x established by element 'col' has no cells beginning
       in it" warnings.
     - Fix "Text run is not in Unicode Normalization Form C".
     - Remove the "type" HTML attribute from <style> elements.
 .
   [ Vibhu ]
   * Fail more gracefully when running out of diskspace. (Closes: #874582)
Checksums-Sha1:
 5a6c110f42ed54a5911b3c20776159afcbcecdab 4533 diffoscope_114.dsc
 a65c8eed454ffb0f76e1114ae73c00f2ed3fdaa6 1117320 diffoscope_114.tar.xz
 3a63794ac70b7e080391f9f8d1fb6f24b2f79ed3 130320 diffoscope_114_all.deb
 e8b49ddf78da84c37a037864356a55c499f10c5d 6504 diffoscope_114_amd64.buildinfo
Checksums-Sha256:
 ce37b6b5eadd8d5ef2c08446ca7deb871797b374d49ed44d30b033aac6f5ee70 4533 diffoscope_114.dsc
 16c881bb0716de6c64ccae72bfecf2cd03661dbe9ab3e9d2f87aae7bd524437b 1117320 diffoscope_114.tar.xz
 07b4d27401e2838fdcfc7a6d2325eacc3e6626cf08207629bb09181ffe41289f 130320 diffoscope_114_all.deb
 3e2e8941377f2e39c04e36cdf25a699350c9fb1cbb396e62e0229b6f4630c964 6504 diffoscope_114_amd64.buildinfo
Files:
 d6eaeb7b3b42987396d562ca71347a8c 4533 devel optional diffoscope_114.dsc
 30e3014327b8fa46feca7e4129a159e7 1117320 devel optional diffoscope_114.tar.xz
 2efdb3d3cd1000bb8d37cf03984dfdea 130320 devel optional diffoscope_114_all.deb
 02aa356dbbf27fa33fe1cd88c59aa57d 6504 devel optional diffoscope_114_amd64.buildinfo

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEwv5L0nHBObhsUz5GHpU+J9QxHlgFAlzF02YACgkQHpU+J9Qx
Hli4rBAAtPEbuXpm68kPLdoWI63CLVa1aazyxCyrpzE4/TiwvuEY8t7MYCWYIACB
a6wyf+BL72pQUW1Gju6R+UHST0uJRN9O5NopuXPejaiyQN+9dQThh/SSaKlAzeyI
3CMoIp3JJsUaAbqJoscvF+l5yLeq+4ogEXhz75ru+UbLwtybnzxPp5bnqHLXI89M
+Vx5eE1TvpoIkEsBP90TSRqGd5t5yFN+jY1tGxyt9e7x8S3p/66nC6ub3U4LDRAk
wUd96hStAv2l690kJFL8/2mkGm1pQvtNovLlYTI4ZDSEZi9uleuKHGlu99uXeZyo
6wMjjMyJbkymQIREssfxgRL3jYW7RCDSsuzU88s19NJKqo7RVMqzWnJCuX84xr2h
WJtbCz+bYH539XEADU4fScMD7iinRv44qd7HCFvsvQrnGQZ++++I5+u9UcVGdS3u
hn7ZaTKn7n77EqdGlLOLW1UgGKnFDVr4HMbsg/OiF419ye0lHUuA/avawKN32AnI
6dnXsNPi2avt7pDGFdLiYVOCyYnZZ6VYy4wPf/I2Dqkl04cYZylWPIc7+SlSA2F/
PgkJNRth0Z9GjNQEStAgWZ+8d/wbWWzgVv6MSz5t/ueJVPsktvnaftBXcxE5Jv9C
bLvzTWhrDlD2XuIZBepe3EA1j1zfKwWA5AK4OpSjvo6i1fZbgfw=
=V/if
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.