Project

General

Profile

Feature #6309

Feature #6298: Write more automated tests

Write tests for incremental upgrades

Added by bertagaz over 6 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
High
Assignee:
-
Category:
Test suite
Target version:
Start date:
09/26/2013
Due date:
07/31/2016
% Done:

100%

Feature Branch:
test/6309-incremental-upgrades
Type of work:
Code
Blueprint:
Starter:
No
Affected tool:

Description

Our test suite should contain tests for this functionnality.

A cheap solution that would cover 90% of this is the following: we create a thrid channel (beyond stable and alpha) that publishes a UDF (that we have to publish for each new version) that points to a simple IUK that simple adds some file to the Tails filesystem, deletes some others, and adds something in the live/ sub-dir, and then we install it and verify that these modifications have been applied.


Related issues

Blocked by Tails - Feature #5922: incremental upgrades: complete phase one Resolved 10/08/2013

Associated revisions

Revision be062878 (diff)
Added by anonym over 3 years ago

Add UDFs used for our upcoming automated test.

Refs: #6309

Revision 267fa4db (diff)
Added by anonym over 3 years ago

Automatically test incremental upgrades.

Will-fix: #6309

Revision ea8ef8cb (diff)
Added by anonym over 3 years ago

Make sure IUK are included in the chroot browser aufs stacks.

Will-fix: #6309

Revision 91aa15f8 (diff)
Added by bertagaz over 3 years ago

Make sure no notifications will interfere with the tests.

Once the network is connected, let just close all notifications so that
the detection of the later ones is more robust and doesn't break the
test.

Refs: #6309

Revision 07d3c6f0 (diff)
Added by bertagaz over 3 years ago

Typo.

Refs: #6309

Revision ca763642 (diff)
Added by anonym over 3 years ago

Retry fetching the incremental upgrade on failure.

Refs: #6309

Revision e03bde9f (diff)
Added by bertagaz over 3 years ago

Catch network errors on incremental upgrades with retry_tor too.

Using the same trick than for the UDF downloading.

Refs: #6309

Revision eb52b865
Added by bertagaz over 3 years ago

Merge branch 'test/6309-incremental-upgrades' into stable

Fix-committed: #6309

History

#1 Updated by BitingBird about 5 years ago

  • Description updated (diff)

#2 Updated by anonym about 5 years ago

  • Target version set to Tails_2.4

#4 Updated by intrigeri over 4 years ago

  • Due date set to 07/31/2016

anonym, kytv: this ticket needs an assignee.

#5 Updated by anonym over 4 years ago

  • Description updated (diff)
  • Assignee set to anonym

#6 Updated by intrigeri over 3 years ago

  • Description updated (diff)

#7 Updated by anonym over 3 years ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10
  • QA Check set to Dev Needed

Let's start thinking about this. Some notes:

  • Let's go with the "cheap" solution from the ticket description since it will excersice all the needed code.
  • As for "where" to publish the UDF, I wonder if we in this new scenario could set the TAILS_VERSION in /etc/os-release to 1.0-test-suite or similar, and then publish the UDF in wiki/src/upgrade/v1/Tails/test/.... Does that have some unexpected consequences? intrigeri?
  • I'm thinking we'd upgrade to version 2.0-test-suite, and the IUK would include the /etc/os-release and /etc/amnesia/version to reflect that. The IUK would be: Tails_i386_1.0-test-suite_to_2.0-test-suite.iuk.
  • Since we have to manually mount the IUK filesystem stack for the chroot browsers, we should test that the fs changes via the IUK is live in the chroot after starting the Unsafe Browser.
  • Would it be worth/sensible to also disable the upgrade check for all other scenarios? At the moment our test suite may break if run on old images, since an upgrade prompt will appear which might disrupt, and it would save a bit of time (very little given our use of snapshots).

#8 Updated by intrigeri over 3 years ago

  • As for "where" to publish the UDF, I wonder if we in this new scenario could set the TAILS_VERSION in /etc/os-release to 1.0-test-suite or similar, and then publish the UDF in wiki/src/upgrade/v1/Tails/test/.... Does that have some unexpected consequences? intrigeri?

Assuming you mean wiki/src/upgrade/v1/Tails/1.0-test-suite/...: it should work, I guess (and we'll see :)

And hopefully we won't ever add other code that depends on the current version.

Adding a new, dedicated "channel" (rather than a special version) might be a bit more elegant, but it implies to patch tails-iuk-generate-upgrade-description-files to automatically create the corresponding UDFs, so it's a bit more work; not needed IMO, go ahead with your idea.

  • I'm thinking we'd upgrade to version 2.0-test-suite, and the IUK would include the /etc/os-release and /etc/amnesia/version to reflect that. The IUK would be: Tails_i386_1.0-test-suite_to_2.0-test-suite.iuk.

Let's say 1.0 → 1.1 (instead of 1.0 → 2.0), so that whenever we need to update the whole thing, we can e.g. do 2.0 → 2.1. But that's very much a detail, I don't care much.

  • Since we have to manually mount the IUK filesystem stack for the chroot browsers, we should test that the fs changes via the IUK is live in the chroot after starting the Unsafe Browser.

Good idea, since we've already had a bug in this area!

  • Would it be worth/sensible to also disable the upgrade check for all other scenarios? At the moment our test suite may break if run on old images, since an upgrade prompt will appear which might disrupt, and it would save a bit of time (very little given our use of snapshots).

No strong opinion. The benefit seems very small, so I'm not sure it's worth adding+maintaining more code. In doubt, I would tend to not add more code, generally ;)

#9 Updated by anonym over 3 years ago

intrigeri wrote:

  • As for "where" to publish the UDF, I wonder if we in this new scenario could set the TAILS_VERSION in /etc/os-release to 1.0-test-suite or similar, and then publish the UDF in wiki/src/upgrade/v1/Tails/test/.... Does that have some unexpected consequences? intrigeri?

Assuming you mean wiki/src/upgrade/v1/Tails/1.0-test-suite/...: it should work, I guess (and we'll see :)

First I considered the version test, and then I forgot to change it.

And hopefully we won't ever add other code that depends on the current version.

If we do, we'll notice. :)

I think I'll go with 1.0~test and 1.1~test just to keep it close to our current versioning format, in case it will ever matter.

  • I'm thinking we'd upgrade to version 2.0-test-suite, and the IUK would include the /etc/os-release and /etc/amnesia/version to reflect that. The IUK would be: Tails_i386_1.0-test-suite_to_2.0-test-suite.iuk.

Let's say 1.0 → 1.1 (instead of 1.0 → 2.0), so that whenever we need to update the whole thing, we can e.g. do 2.0 → 2.1. But that's very much a detail, I don't care much.

Agreed (as you can see above).

  • Would it be worth/sensible to also disable the upgrade check for all other scenarios? At the moment our test suite may break if run on old images, since an upgrade prompt will appear which might disrupt, and it would save a bit of time (very little given our use of snapshots).

No strong opinion. The benefit seems very small, so I'm not sure it's worth adding+maintaining more code. In doubt, I would tend to not add more code, generally ;)

Sure, let's not complicate things.

Ok, with all these things decided, the "infra" work (preparing the UDFs, generatig + uploading the IUK) and writing the test should be very straghtforward! I expected this test to be much harder. Yay! :)

#10 Updated by intrigeri over 3 years ago

  • Priority changed from Normal to High

#11 Updated by anonym over 3 years ago

  • % Done changed from 10 to 40
  • Feature Branch set to test/6309-incremental-upgrades

Let's see what jenkins thinks. Future link: https://jenkins.tails.boum.org/job/test_Tails_ISO_test-6309-incremental-upgrades/1/

The added scenario relies on wait_until_tor_is_working so it should probably be marked @fragile given #10497, but that ticket is supposedly fixed (just not review'n'merged) thanks to chutney, so I guess I can sneak it through without (or review'n'merge #10497 before sending this onr for review).

#12 Updated by anonym over 3 years ago

  • % Done changed from 40 to 30

Ah, just realized that I haven't implemented a test for the chroot browser aufs stack. TODO++.

#13 Updated by anonym over 3 years ago

  • Assignee changed from anonym to intrigeri
  • % Done changed from 30 to 50
  • QA Check changed from Dev Needed to Ready for QA

anonym wrote:

Let's see what jenkins thinks. Future link: https://jenkins.tails.boum.org/job/test_Tails_ISO_test-6309-incremental-upgrades/1/

Never mind this; the scenario was added to a feature which is marked @fragile (due to #10720). It runs well locally for me, though.

Ah, just realized that I haven't implemented a test for the chroot browser aufs stack.

Implemented!

#14 Updated by anonym over 3 years ago

  • Assignee changed from intrigeri to bertagaz

Actually bert was supposed this.

#15 Updated by bertagaz over 3 years ago

  • Target version changed from Tails_2.4 to Tails_2.5

#16 Updated by bertagaz over 3 years ago

  • Assignee changed from bertagaz to anonym
  • % Done changed from 50 to 60
  • QA Check changed from Ready for QA to Info Needed

anonym wrote:

Let's see what jenkins thinks. Future link: https://jenkins.tails.boum.org/job/test_Tails_ISO_test-6309-incremental-upgrades/1/

The added scenario relies on wait_until_tor_is_working so it should probably be marked @fragile given #10497, but that ticket is supposedly fixed (just not review'n'merged) thanks to chutney, so I guess I can sneak it through without (or review'n'merge #10497 before sending this onr for review).

I've run it several hundred of times, and pushed a tiny 91aa15f that fixes a robustness issue: the notifications were sometimes hiding the application under test (unsafe browser or upgrader). With this commit, every notifications are cleared before the real test begins.

I've seen another issue, which may be related to the wait_until_tor_is_working, but I wonder... There are two places were I've seen failure because of network errors: when the upgrader can't fetch the UDF, and when it can't fetch the IUK itself. For the first one, a warning window is opened asking for the user to restart Tails. This case is not handled by the test suite. For the second, the test suite waits and then fail.

OTOH I wonder of it's not more of a UX issue than a test suite one. Like for #10494, maybe the Tails upgrader should retry when it failed to fetch something. So I'd be tempted to propose to merge this branch, because the scenario looks good to me, but open a new ticket about the network errors that should be handled by the upgrader and tag this new incremental upgrade scenario as fragile as long as this other new ticket is not solved. What do you think?

#17 Updated by anonym over 3 years ago

  • Assignee changed from anonym to bertagaz
  • QA Check changed from Info Needed to Ready for QA

bertagaz wrote:

anonym wrote:

Let's see what jenkins thinks. Future link: https://jenkins.tails.boum.org/job/test_Tails_ISO_test-6309-incremental-upgrades/1/

The added scenario relies on wait_until_tor_is_working so it should probably be marked @fragile given #10497, but that ticket is supposedly fixed (just not review'n'merged) thanks to chutney, so I guess I can sneak it through without (or review'n'merge #10497 before sending this onr for review).

I've run it several hundred of times, and pushed a tiny 91aa15f that fixes a robustness issue: the notifications were sometimes hiding the application under test (unsafe browser or upgrader). With this commit, every notifications are cleared before the real test begins.

Thanks!

I've seen another issue, which may be related to the wait_until_tor_is_working, but I wonder... There are two places were I've seen failure because of network errors: when the upgrader can't fetch the UDF, and when it can't fetch the IUK itself. For the first one, a warning window is opened asking for the user to restart Tails. This case is not handled by the test suite. For the second, the test suite waits and then fail.

Ok, so the test suite should retry, then. Pushed in ca76364. You can easily test the retry-code by changing the version in the Tails is fooled to think it is running version step in the .feature to some non-existing Tails version.

OTOH I wonder of it's not more of a UX issue than a test suite one. Like for #10494, maybe the Tails upgrader should retry when it failed to fetch something. So I'd be tempted to propose to merge this branch, because the scenario looks good to me, but open a new ticket about the network errors that should be handled by the upgrader and tag this new incremental upgrade scenario as fragile as long as this other new ticket is not solved. What do you think?

Let's not add tests that are @fragile to begin with. :) Still, I do agree that this is a bug in Tails Upgrader, and should have a ticket (but IIRC there is one already).

#18 Updated by bertagaz over 3 years ago

  • Assignee changed from bertagaz to anonym
  • % Done changed from 60 to 70

The errors catching in ca76364 works fine according to my numerous test runs. I was ready to merge that branch, but I've seen failure where the UDF download went fine (after some retries), but the IUK download failed, making the scenario fail.

So I've pushed a last e03bde9 on the branch that catches this error, inspired from the previously mentioned commit. I've tested and seen it recovering this error. It may have deserved a bit more refactoring, but it works well as it is. Re-assigning to anonym to review this commit and merge the branch if likes it.

#19 Updated by intrigeri over 3 years ago

  • Assignee changed from anonym to intrigeri

#20 Updated by intrigeri over 3 years ago

  • Assignee changed from intrigeri to anonym
  • Target version changed from Tails_2.5 to Tails_2.6
  • QA Check changed from Ready for QA to Dev Needed

I'm not merging this since I'm not convinced that e03bde9 is good enough. What it does is that in:

    Then I am proposed to install an incremental upgrade to version 1.1~test
    When I agree to install the incremental upgrade
    Then the incremental upgrade to version "1.1~test" is reportedly installed

... it adds retrying logic for "I am proposed to install […]" and "I agree to install the incremental upgrade" in "the incremental upgrade to version "1.1~test" is reportedly installed", which feels wrong semantically speaking, and will lead to awkward experiences next time someone has to debug this part of the test suite. Such a "Then" step should merely be about verifying that everything is in order, not about retrying what the previous "When" step tried to do. I'm not sure how to fix this. E.g. "I agree to install the incremental upgrade" could be renamed to "I agree to install the incremental upgrade and wait until it is applied", and implement the retrying logic? Or merge the When and Then steps into a single one whose name makes it clear that it also checks for success?

"I agree to install the incremental upgrade" somewhat suffers from the same problem, but there it's acceptable IMO, since we don't explicitly have a previous "When" step is our Gherkin text.

Other than that, code review passes for me too (but really, who cares, since everything before the commit discussed above was already reviewed by someone). I'll report back about local testing (started before I reviewed the last commit so I can as well let it complete) later, if I see any problem with it.

#21 Updated by anonym over 3 years ago

  • Assignee changed from anonym to bertagaz
  • % Done changed from 70 to 80
  • QA Check changed from Dev Needed to Ready for QA

I've now fixed up the Gherkin, let me know what you think.

I also sneaked in aa7b2153bad63f56599b823192bdadbbd7fcb1db which is a quality of life improvement for us test suite developers, and that will not affect runs without --keep-snapshots at all, so it can be merged without any worry of messing stuff up on Jenkins.

bert, will you be able to look at the two new commits in time for Tails 2.6?

#22 Updated by anonym over 3 years ago

  • Target version changed from Tails_2.6 to Tails_2.7

#23 Updated by bertagaz over 3 years ago

  • Status changed from In Progress to 11
  • Assignee deleted (bertagaz)
  • % Done changed from 80 to 100
  • QA Check changed from Ready for QA to Pass

anonym wrote:

I've now fixed up the Gherkin, let me know what you think.

Works for me!

I also sneaked in aa7b2153bad63f56599b823192bdadbbd7fcb1db which is a quality of life improvement for us test suite developers, and that will not affect runs without --keep-snapshots at all, so it can be merged without any worry of messing stuff up on Jenkins.

Ok, that's sneaky but good enough to be merged with the branch.

bert, will you be able to look at the two new commits in time for Tails 2.6?

Sorry I didn't, but just merged it in the end, congrats!

#24 Updated by sajolida over 3 years ago

  • Related to Bug #11857: Provide IUK to go from version n-2 to version n added

#25 Updated by sajolida over 3 years ago

I didn't follow this closely but this sounds super tricky and cool! Congrats!

Does this mean that providing more IUKs has now become less costly at release time? I'm thinking about #10478#note-3. But let's move this discussion to #11857.

#26 Updated by intrigeri over 3 years ago

Does this mean that providing more IUKs has now become less costly at release time? I'm thinking about #10478#note-3.

I think that's entirely unrelated.

#27 Updated by anonym over 3 years ago

intrigeri wrote:

Does this mean that providing more IUKs has now become less costly at release time? I'm thinking about #10478#note-3.

I think that's entirely unrelated.

Yup, unrelated. In this test we cheat and use the same hand-crafted IUK no matter which Tails version we are testing from.

#28 Updated by bertagaz about 3 years ago

  • Status changed from 11 to Resolved

#29 Updated by intrigeri about 3 years ago

  • Related to deleted (Bug #11857: Provide IUK to go from version n-2 to version n)

Also available in: Atom PDF