Feature #6309

Write tests for incremental upgrades

Added by bertagaz 2013-09-26 05:50:11 . Updated 2016-11-15 18:23:31 .

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

100%

Feature Branch:
test/6309-incremental-upgrades
Type of work:
Code
Blueprint:

Starter:
0
Affected tool:
Deliverable for:
270

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.

Subtasks

Related issues

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

History

#1 Updated by BitingBird 2015-01-02 19:12:42

  • Description updated

#2 Updated by anonym 2015-01-10 05:58:02

  • Target version set to Tails_2.4

#3 Updated by anonym 2015-01-10 17:19:00

  • blocks #8668 added

#4 Updated by intrigeri 2015-08-02 11:09:21

  • Due date set to 2016-07-31

anonym, kytv: this ticket needs an assignee.

#5 Updated by anonym 2015-09-26 08:51:21

  • Description updated
  • Assignee set to anonym

#6 Updated by intrigeri 2016-05-10 07:43:55

  • Description updated

#7 Updated by anonym 2016-05-13 10:35:47

  • 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 2016-05-14 06:05:47

> * 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 2016-05-14 06:33:55

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 2016-05-18 13:19:53

  • Priority changed from Normal to High

#11 Updated by anonym 2016-05-31 21:08:32

  • % 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 Bug #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 Bug #10497 before sending this onr for review).

#12 Updated by anonym 2016-05-31 21:11:02

  • % 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 2016-06-01 11:38:33

  • 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 Bug #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 2016-06-01 11:53:27

  • Assignee changed from intrigeri to bertagaz

Actually bert was supposed this.

#15 Updated by bertagaz 2016-06-07 12:31:33

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

#16 Updated by bertagaz 2016-06-27 03:44:00

  • 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 Bug #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 Bug #10497 before sending this onr for review).

I’ve run it several hundred of times, and pushed a tiny commit: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 Bug #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 2016-07-01 07:18:50

  • 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 Bug #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 Bug #10497 before sending this onr for review).
>
> I’ve run it several hundred of times, and pushed a tiny commit: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 commit: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 Bug #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 2016-07-04 05:41:38

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

The errors catching in commit: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 commit: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 2016-07-21 06:01:39

  • Assignee changed from anonym to intrigeri

#20 Updated by intrigeri 2016-07-21 08:44:11

  • 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 commit: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 2016-09-16 06:14:59

  • 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 commit: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 2016-09-20 16:53:37

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

#23 Updated by bertagaz 2016-09-30 08:31:23

  • Status changed from In Progress to Fix committed
  • 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 commit: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 2016-10-02 17:49:44

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

#25 Updated by sajolida 2016-10-02 17:50:09

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 Feature #10478#note-3. But let’s move this discussion to Bug #11857.

#26 Updated by intrigeri 2016-10-03 07:52:31

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

I think that’s entirely unrelated.

#27 Updated by anonym 2016-10-03 10:54:51

intrigeri wrote:
> > Does this mean that providing more IUKs has now become less costly at release time? I’m thinking about Feature #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 2016-11-15 18:23:31

  • Status changed from Fix committed to Resolved

#29 Updated by intrigeri 2017-01-02 17:58:50

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