Tails

#1 Updated by anonym 2016-01-03 13:53:45

• Assignee set to kytv

kytv, please set an appropriate milestone/target.

#2 Updated by intrigeri 2016-01-03 14:30:54

• related to Bug #10784: Dotfiles persistent feature is broken on Jessie added

#3 Updated by intrigeri 2016-01-03 14:32:52

• Target version set to Tails_2.3
• Parent task set to Feature #9476
• Deliverable for set to 270

(Setting metadata tentatively so there’s no chance it is forgotten, but please update so that it integrates into your roadmap.)

#4 Updated by bertagaz 2016-03-03 15:40:36

• Assignee changed from kytv to bertagaz

Taking over this ticket.

#5 Updated by bertagaz 2016-03-03 15:41:33

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

#6 Updated by intrigeri 2016-03-03 16:00:25

• Parent task changed from Feature #9476 to Feature #9477

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

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

#8 Updated by bertagaz 2016-06-29 06:25:48

• Status changed from Confirmed to In Progress
• % Done changed from 0 to 20
• Feature Branch set to bugfix/10840-automatically-test-dotfiles-persistence

Pushed a branch with a patch to test regression on Bug #10784. Sadly it won’t be tested by Jenkins, because the whole feature is tagged fragile because of Bug #10720.

#9 Updated by bertagaz 2016-06-30 10:45:00

• Assignee deleted (bertagaz)
• QA Check set to Ready for QA

Well, that code runs fine at home, and is so light that I don’t think it’s worth waiting more. Someone has to look at it and run it but I’m not sure who. Any takers?

#10 Updated by intrigeri 2016-07-09 07:16:28

• Assignee set to intrigeri

In theory that would be on anomym’s plate, but I’m ready to help.

#11 Updated by intrigeri 2016-07-13 10:34:04

• Assignee changed from intrigeri to bertagaz
• % Done changed from 20 to 0
• QA Check changed from Ready for QA to Dev Needed

There seems to be a misunderstanding.

IMO the regression test we want here is “automatically test dotfiles persistence”, and is about testing that the functionality keeps working (integration test), not about checking one specific way in which it could be broken (unit test). In particular, because the specific way it’s been broken in the past has little chances to occur again (so the added test suite code is mostly useless, but still needs to be maintained), while the general fragileness of this feature makes it likely to break in other ways in the future (so an integration test would be worthwhile).

I’m open to hear your arguments and discuss this further if you disagree.

#13 Updated by bertagaz 2016-07-23 10:24:10

• Assignee changed from bertagaz to intrigeri
• QA Check changed from Dev Needed to Info Needed

So I’ve reverted the previous commit, updated the branch to the latest stable, and pushed commit:8307257 which implements the dotfiles feature test. Works fine at home.

I’m not sure it’s the best direction, but I thought adding a whole scenario for it was a bit too much overhead: it would have mean yet another scenario with multiple boots of Tails. So I’ve hooked it along with other persistence tests, but the place where it was easiest to integrate it is in the usb_upgrade feature, and on this I’m unsure. Re-assigning for input on that. Also is there other areas of the dotfiles we’d need to test, or is it already enough?

#14 Updated by intrigeri 2016-07-23 11:36:40

• Assignee changed from intrigeri to bertagaz
• QA Check changed from Info Needed to Dev Needed

> pushed commit:8307257 which implements the dotfiles feature test. Works fine at home.

That’s a pretty good start! You’ll want to spell check “persitent” though.

> I’m not sure it’s the best direction, but I thought adding a whole scenario for it was a bit too much overhead: it would have mean yet another scenario with multiple boots of Tails. So I’ve hooked it along with other persistence tests, but the place where it was easiest to integrate it is in the usb_upgrade feature, and on this I’m unsure.

Looks like premature optimization to me. Let’s please bother about optimization only once we get something that feels right (including from the Gherkin PoV, which is what matters most), works, and proves to affect performance too badly.

> is there other areas of the dotfiles we’d need to test, or is it already enough?

I think that’s enough :)

#15 Updated by bertagaz 2016-07-24 05:43:22

• Assignee changed from bertagaz to intrigeri
• % Done changed from 0 to 30
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> That’s a pretty good start! You’ll want to spell check “persitent” though.
>
> Looks like premature optimization to me. Let’s please bother about optimization only once we get something that feels right (including from the Gherkin PoV, which is what matters most), works, and proves to affect performance too badly.

Ok, then I’ve moved this test into its own scenario (and fixed the typo) with commit commit:80b2efe5. It takes somewhere between 15 and 20 minutes on my hardware.

#16 Updated by intrigeri 2016-07-25 02:48:07

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

> Ok, then I’ve moved this test into its own scenario (and fixed the typo) with commit commit:80b2efe5.

OK, that’s better!

I just have a few comments on the implementation:

• Including “is working” in the scenario’s name is not consistent with the rest of our test suite, and is useless AFAICT.
• I see one more “persitence” introduced in that commit.
• I don’t get why you’re stuffing the new code into the “I write some files expected to persist” function, while not a single line of code is shared between its previous implementation and the additional one you added in there. Just add a “I write some dotfile expected to persist” When function instead? Same question for “the expected persistent files”.

> It takes somewhere between 15 and 20 minutes on my hardware.

Just to clarify: when run as part of the complete persistence.feature, i.e. with some of the intermediary state restored from snapshots? Or when run in isolation?

#17 Updated by bertagaz 2016-07-25 04:08:07

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

intrigeri wrote:
> * Including “is working” in the scenario’s name is not consistent with the rest of our test suite, and is useless AFAICT.

Ack.

> * I see one more “persitence” introduced in that commit.

Erf, apologies, seems my fingers can’t get to type it correctly (nor my brain to catch their mistake).

> * I don’t get why you’re stuffing the new code into the “I write some files expected to persist” function, while not a single line of code is shared between its previous implementation and the additional one you added in there. Just add a “I write some dotfile expected to persist” When function instead? Same question for “the expected persistent files”.

Because at the Gherkin level, the difference is so tight (1 word) that I thought to factorize it.

> > It takes somewhere between 15 and 20 minutes on my hardware.
>
> Just to clarify: when run as part of the complete persistence.feature, i.e. with some of the intermediary state restored from snapshots? Or when run in isolation?

Right, that’s without snapshots.

Anyway, pushed commit commit:bec68ea

#19 Updated by intrigeri 2016-07-26 04:15:08

• % Done changed from 30 to 60

#23 Updated by intrigeri 2016-07-26 11:41:12

• Status changed from In Progress to Fix committed
• Assignee deleted (intrigeri)
• % Done changed from 60 to 100
• QA Check changed from Ready for QA to Pass

Merged!

#24 Updated by intrigeri 2016-08-02 09:29:47

• Status changed from Fix committed to Resolved