Feature #14817
Test suite improvements from buster sprint 1
100%
Description
This includes the fabled --fuzzy-image-matching
option for the automated test suite, among a few other minor fixes.
Please review’n’merge into devel.
Subtasks
Related issues
Blocks Tails - |
Resolved | 2017-06-29 | |
Blocked by Tails - |
Resolved | 2017-11-29 | |
Blocked by Tails - |
Resolved | 2017-11-07 | |
Blocks Tails - |
Resolved | 2017-10-22 |
History
#1 Updated by intrigeri 2017-10-09 12:32:22
- Target version changed from Tails_3.5 to Tails_3.6
#2 Updated by intrigeri 2017-10-09 13:03:46
- Status changed from Confirmed to Fix committed
- % Done changed from 50 to 100
Applied in changeset commit:7c92cec5853d88c2b96d6e93614a21cd35e8c700.
#3 Updated by intrigeri 2017-10-09 13:34:39
- Assignee deleted (
intrigeri) - QA Check changed from Ready for QA to Pass
Merged into devel & feature/buster. thanks!
#4 Updated by intrigeri 2017-10-10 05:11:52
- Status changed from Fix committed to In Progress
- Assignee set to anonym
- Priority changed from Normal to Elevated
- % Done changed from 100 to 70
- QA Check changed from Pass to Dev Needed
Two issues noticed after merging:
- This seems to break
maybe_deal_with_pinentry
when the passphrase is cached, seeFeature #14814#note-2. I suspect that’ll break stuff on the devel branch too so raising priority. We can’t see it on Jenkins as the affected tests are flagged as fragile. - The candidate pics are not saved as artifacts on Jenkins => please file a ticket about it, assigned to me.
#5 Updated by intrigeri 2017-10-10 05:12:16
- blocks
Feature #13240: Core work 2017Q4: Test suite maintenance added
#6 Updated by intrigeri 2017-10-10 05:12:30
- Target version changed from Tails_3.6 to Tails_3.3
#7 Updated by anonym 2017-10-11 16:19:21
- Assignee changed from anonym to intrigeri
- % Done changed from 70 to 80
- QA Check changed from Dev Needed to Ready for QA
intrigeri wrote:
> Two issues noticed after merging:
>
> * This seems to break maybe_deal_with_pinentry
when the passphrase is cached, see Feature #14814#note-2. I suspect that’ll break stuff on the devel branch too so raising priority. We can’t see it on Jenkins as the affected tests are flagged as fragile.
Your suspicious from Feature #14814 was not the case; the nature of the error turned out to be much more arcane, about how Java exceptions are integrated into Ruby via rjb. Fixed in commit:f2744487a2e0d4d52554c516e6f1d25d3f3c32e3.
While investigating your suspicion, I realized findAny()
needed some love too (now that some code runs unconditionally in findfailed_hook()
— previously it only caused breakage in combination with --retry-find
), see commit:431be2d59bc4677de87f8ee8cc226b58662e63a1.
> * The candidate pics are not saved as artifacts on Jenkins => please file a ticket about it, assigned to me.
Filed as Feature #14830.
#8 Updated by intrigeri 2017-10-15 05:52:13
> Your suspicious from Feature #14814 was not the case; the nature of the error turned out to be much more arcane, about how Java exceptions are integrated into Ruby via rjb. Fixed in commit:f2744487a2e0d4d52554c516e6f1d25d3f3c32e3.
Wow. Glad you found it :)
> While investigating your suspicion, I realized findAny()
needed some love too (now that some code runs unconditionally in findfailed_hook()
— previously it only caused breakage in combination with --retry-find
), see commit:431be2d59bc4677de87f8ee8cc226b58662e63a1.
OK.
It would take me a long time to dive back into all this and do a proper review, but this being said the diff doesn’t look insane. Will test and merge if it works for me :)
#9 Updated by intrigeri 2017-10-16 07:30:58
- Assignee changed from intrigeri to anonym
- QA Check changed from Ready for QA to Dev Needed
This fixed the OpenPGP applet tests for me on feature/buster + this branch merged. Woohoo! :) Only the two following issues prevent me from merging the updated branch.
On feature/buster + this branch merged, I see a bunch of errors I’ve never seen before e.g.:
Then I am prompted to configure Electrum # features/step_definitions/electrum.rb:44
01:32:40.940001822: Found fuzzy candidate picture for ElectrumNextButton.png with similarity 0.7
When I create a new bitcoin wallet # features/step_definitions/electrum.rb:17
undefined method `toString' for nil:NilClass (NoMethodError)
features/electrum.feature:25:in `When I create a new bitcoin wallet'
I saw that in 4 Tor Browser scenarios too. Any chance this is a regression brought by this branch?
One weird thing: apparently we save Sikuli candidates even when we ended up finding the correct picture on screen (and fuzzy image matching is disabled) e.g.:
When I request a reboot using the emergency shutdown applet # features/step_definitions/common_steps.rb:553
Slept for 5 seconds
02:00:34.252477979: Failed to find fuzzy candidate picture for TailsGreeter.png
02:00:55.235262140: Failed to find fuzzy candidate picture for TailsGreeter.png
02:01:00.013057547: Found fuzzy candidate picture for TailsGreeter.png with similarity 0.8
Then Tails eventually restarts # features/step_definitions/common_steps.rb:522
How about we keep and publish these only when there’s a chance we need them?
#10 Updated by intrigeri 2017-10-22 08:25:03
- Status changed from In Progress to Fix committed
- % Done changed from 80 to 100
Applied in changeset commit:13fe64b2c6cdbfc56d85b23852cc51841c5d9e14.
#11 Updated by intrigeri 2017-10-22 08:40:18
- Status changed from Fix committed to In Progress
- % Done changed from 100 to 80
(Grml, I guess this will happen every time we merge devel into anything else.)
#12 Updated by intrigeri 2017-11-10 14:16:33
- Target version changed from Tails_3.3 to Tails_3.5
#13 Updated by anonym 2017-11-30 18:07:32
- Assignee changed from anonym to intrigeri
- QA Check changed from Dev Needed to Ready for QA
intrigeri wrote:
> This fixed the OpenPGP applet tests for me on feature/buster + this branch merged. Woohoo! :) Only the two following issues prevent me from merging the updated branch.
>
> On feature/buster + this branch merged, I see a bunch of errors I’ve never seen before e.g.:
>
> […]
>
> I saw that in 4 Tor Browser scenarios too. Any chance this is a regression brought by this branch?
Yes! Fixed in commit:6a2ec77c1bb18029bd691ab4799ebd5193f7bb3a.
> One weird thing: apparently we save Sikuli candidates even when we ended up finding the correct picture on screen (and fuzzy image matching is disabled) e.g.:
>
> […]
>
> How about we keep and publish these only when there’s a chance we need them?
That is the intention, so this must be a bug. From your log it’s unclear whether the following step (Then Tails eventually restarts
) succeeds or fails, which matters:
On failure: then it is just a matter of logging things incorrectly, which is fixed in commit:afdbec3efe9f32f6facfed31656c9ca96d216ecf.
On success: this could be related to the issue I just fixed with commit:6a2ec77c in a very nasty way: I noticed that using Rjb to directly throwing Java exceptions does nothing. Imagine if the bug that prevents Java exceptions from being re-raised isn’t completely broken, just race:y and sometimes manages to enter the Java exception throwing code; well, since that code does nothing, then findfailed_hook
returns, and the Sikuli method succeeds (which is completely incorrect here!), and that is the very last line of code for this whole feature. Any way, with commit:6a2ec77c we avoid both bugs, so this should not be an issue any more.
So it might be fixed. At least I fail to reproduce it myself.
#14 Updated by anonym 2017-12-04 10:04:30
- Feature Branch changed from test/devel-improvements-from-buster-sprint to test/improvements-from-buster-sprint
Actually, I’d like these changes in the stable
branch (waiting to March for all stable
-based branches seems like potentially wasteful) so I’ve rebased the branch. Note: this will actually eliminate the test suite delta between stable
and devel
(the only other change currently in devel
being Bug #14772).
I left the original test/devel-improvements-from-buster-sprint
around for comparison with this new rebased branch.
#15 Updated by intrigeri 2017-12-04 10:10:11
> Actually, I’d like these changes in the stable
branch (waiting to March for all
> stable
-based branches seems like potentially wasteful) so I’ve rebased the branch.
Glad I took my sweet time to review then :)
#16 Updated by anonym 2017-12-04 15:01:37
BTW, I can deal with the stable
→ devel
merge, which will contain non-trivial (but still easy, I think) conflicts.
#17 Updated by anonym 2017-12-04 15:06:14
anonym wrote:
> BTW, I can deal with the stable
→ devel
merge, which will contain non-trivial (but still easy, I think) conflicts.
Actually the resolution should simply be: git checkout --theirs
#18 Updated by intrigeri 2017-12-05 14:42:21
- blocked by
Bug #15007: "Pidgin's D-Bus interface is not available" test suite step fails added
#19 Updated by intrigeri 2017-12-05 14:42:42
- blocked by Bug #15006: Test suite regression: "I successfully start the Unsafe Browser" fails since Tails 3.3 added
#20 Updated by intrigeri 2017-12-05 14:48:00
- blocked by
Bug #14929: 'I can view a file stored in "~/Tor Browser" but not in ~/.gnupg' scenario is broken added
#21 Updated by intrigeri 2017-12-05 14:56:20
- Assignee changed from intrigeri to anonym
- QA Check changed from Ready for QA to Info Needed
I’ve run the full test suite on this branch and:
- lots of failures caused by known recent regressions in our test suite (see tickets I’ve marked as blocking this one); this prevents me from evaluating how this branch works wrt. a baseline since we have no relevant baseline currently. anonym, do you want to batch-fix these issues (on this branch or directly in stable)? Otherwise just tell me and I’ll do it, so I can do this review in better conditions (no strict ETA though, I’ll take it easy :)
I can print the current page as "output.pdf" to the default downloads directory'
andI can print the current page as "output.pdf" to the persistent Tor Browser directory
fail. I’ll investigate but I doubt this has anything to do with this branch.
I still need to verify that the regressions I had previously reported have indeed been fixed by your recent commits. I’m confident!
#22 Updated by intrigeri 2017-12-08 17:57:08
- blocks
Bug #14874: "Failing to add a certificate to Pidgin" test scenario always fails on devel since Sikuli fuzzy matching was implemented added
#23 Updated by intrigeri 2017-12-09 06:23:59
- blocks deleted (
Bug #15006: Test suite regression: "I successfully start the Unsafe Browser" fails since Tails 3.3)
#24 Updated by intrigeri 2017-12-09 06:40:54
- % Done changed from 0 to 70
- QA Check changed from Info Needed to Ready for QA
After a full test suite run, I’m happy to tell that the only blocker left is Bug #14929: feel free to merge once you’ve seen that scenario pass on this topic branch.
#25 Updated by anonym 2017-12-09 14:14:03
- Assignee changed from anonym to intrigeri
[Gah, I forgot to push “Submit” yesterday]
intrigeri wrote:
> * lots of failures caused by known recent regressions in our test suite (see tickets I’ve marked as blocking this one); this prevents me from evaluating how this branch works wrt. a baseline since we have no relevant baseline currently. anonym, do you want to batch-fix these issues (on this branch or directly in stable)? Otherwise just tell me and I’ll do it, so I can do this review in better conditions (no strict ETA though, I’ll take it easy :)
All of these should be fixed now!
> * I can print the current page as "output.pdf" to the default downloads directory'
and I can print the current page as "output.pdf" to the persistent Tor Browser directory
fail. I’ll investigate but I doubt this has anything to do with this branch.
This is a regression introduced in Tails 3.3, filed as Bug #15024
> I still need to verify that the regressions I had previously reported have indeed been fixed by your recent commits. I’m confident!
#26 Updated by intrigeri 2017-12-11 11:20:31
I’ve started another full test suite run (this is @fragile so Jenkins does not help) with a branch based on current stable to double-check.
#27 Updated by intrigeri 2017-12-13 12:53:42
- Status changed from In Progress to Fix committed
- Assignee deleted (
intrigeri) - % Done changed from 70 to 100
- QA Check changed from Ready for QA to Pass
#28 Updated by intrigeri 2018-01-04 18:26:37
- Target version changed from Tails_3.5 to Tails_3.4
#29 Updated by anonym 2018-01-09 20:55:27
- Status changed from Fix committed to Resolved