Tails

#1 Updated by intrigeri 2018-08-08 05:47:22

• blocks Feature #13241: Core work: Test suite maintenance added

#2 Updated by intrigeri 2018-08-08 06:37:53

• Status changed from Confirmed to In Progress
• % Done changed from 0 to 10
• Feature Branch set to test/15771-use-reliable-gnupg-key+force-all-tests

#3 Updated by intrigeri 2018-08-08 11:28:39

• Assignee changed from intrigeri to segfault
• % Done changed from 10 to 50
• Estimated time set to 0 h
• QA Check set to Ready for QA

torified_gnupg.feature passes again. Could you do a quick code review to ensure I did not do anything stupid or dangerous?

#4 Updated by segfault 2018-08-14 14:55:07

• Assignee changed from segfault to intrigeri
• QA Check changed from Ready for QA to Dev Needed

Code review passes, except for one thing:

IIUC, the two tests containing `But the key “0EE5BE979282D80B9F7540F1CCD2ED94D21739E9” has only 10 signatures` will not run if the key does not have exactly 10 self-signatures. While this is currently the case, it could (and probably will) change in the future. This was also the case before your change, but then at least the key was under our (anonym’s) control.

Possible solution: Add a “less then” parameter, and use it with a high enough number (50?) that won’t be reached by accident. I suppose we can trust dkg to not make our tests buggy on purpose by adding 50 self-signatures to his key :)

#6 Updated by intrigeri 2018-08-15 07:24:39

• Assignee changed from intrigeri to segfault
• QA Check changed from Dev Needed to Ready for QA

#7 Updated by segfault 2018-08-15 07:47:37

• Assignee changed from segfault to intrigeri
• QA Check changed from Ready for QA to Pass

intrigeri wrote:
> > This was also the case before your change, but then at least the key was under our (anonym’s) control.
>
> … which did not help much, otherwise I would not have had to file this ticket in the first place.

It doesn’t seem like that was the problem, the key still has 2 self-signed signatures.

> > Possible solution: Add a “less then” parameter, and use it with a high enough number (50?) that won’t be reached by accident. I suppose we can trust dkg to not make our tests buggy on purpose by adding 50 self-signatures to his key :)
>
> Good idea. I’ve implemented this, now waiting for our (very overloaded today) Jenkins CI to validate it. I’m quite confident it will work (famous last words) so you might want to review it already: commit:8f3f506446039ac9e4ed2c7083d1e36acd64cd50.

Code review passes

#8 Updated by intrigeri 2018-08-15 08:44:44

• Status changed from In Progress to Fix committed

>> … which did not help much, otherwise I would not have had to file this ticket in the first place.

> It doesn’t seem like that was the problem, the key still has 2 self-signed signatures.

I believe you’re mistaken and your using “still” word explains why:

• If you count invalid (expired) self-sigs, anonym’s key has had way more than 2 self-sigs for years; the test with the hard-coded “2” used to work only because this test we use --keyserver-options import-clean, so expired self-sigs are not imported.
• If you don’t count invalid (expired) self-sigs, before his key expired, he indeed had 2 valid self-sigs; then after it expired, it had 0, which made tests fail and thus I’ve filed this ticket; and finally, since he updated his key, as of today the key has 2 valid self-sigs again.

> Code review passes

Thanks, merged!

#9 Updated by intrigeri 2018-08-15 08:44:57

• Assignee deleted (intrigeri)
• % Done changed from 50 to 100

#10 Updated by intrigeri 2018-08-16 09:14:33

• Status changed from Fix committed to In Progress

Applied in changeset commit:cd2932ecc1b77883c506da1a9ef1544bae9c702a.

#11 Updated by intrigeri 2018-08-16 09:14:34

• Status changed from In Progress to Fix committed

Applied in changeset commit:1a6f57b15a225fd7bc24401a4282562f9efbf8b8.

#12 Updated by intrigeri 2018-09-05 16:16:46

• Status changed from Fix committed to Resolved