Tails

#1 Updated by kytv 2015-11-06 11:27:06

• Target version set to Tails_1.8

#2 Updated by kytv 2015-11-06 12:43:06

• Feature Branch set to kytv/test/10498-ssh-tests-are-fragile

#3 Updated by kytv 2015-11-08 07:58:46

• Status changed from Confirmed to In Progress
• % Done changed from 0 to 40

I’m still testing but so far my results are very promising. :)

#4 Updated by kytv 2015-11-09 03:30:10

• Assignee changed from kytv to anonym
• % Done changed from 40 to 50
• QA Check set to Ready for QA

#5 Updated by intrigeri 2015-12-05 13:26:12

• Assignee changed from anonym to intrigeri

I’ll give it a try.

#6 Updated by bertagaz 2015-12-05 13:47:59

• Assignee changed from intrigeri to bertagaz

intrigeri wrote:
> I’ll give it a try.

We should really do collective plates rather than stealing each others one! :)

#7 Updated by bertagaz 2015-12-15 03:25:58

• Target version changed from Tails_1.8 to Tails_2.0

Postponing

#8 Updated by kytv 2015-12-16 13:33:21

• Assignee changed from bertagaz to kytv

This will need to be updated for Jessie.

#9 Updated by kytv 2015-12-16 13:33:34

• QA Check changed from Ready for QA to Dev Needed

#11 Updated by intrigeri 2016-01-06 13:57:47

• Target version changed from Tails_2.0 to Tails_2.2

#12 Updated by intrigeri 2016-02-05 14:55:19

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

intrigeri wrote:
> It would be fine to postpone this to 2.2 or something. In any case, please prioritize your SponsorsM4 stuff (Icedove) higher.

Still the case.

#13 Updated by bertagaz 2016-03-03 15:45:55

• Assignee changed from kytv to bertagaz
• Target version changed from Tails_2.3 to Tails_2.4

#14 Updated by bertagaz 2016-06-07 12:31:35

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

#15 Updated by bertagaz 2016-07-08 11:46:12

• Assignee changed from bertagaz to anonym
• % Done changed from 50 to 70
• QA Check changed from Dev Needed to Ready for QA
• Feature Branch changed from kytv/test/10498-ssh-tests-are-fragile to test/10498-ssh-tests-are-fragile

SSH tests had their retry_tor wrapping done without much troubles, but I’ve finally been able to implement something robust for the SFTP scenario, which was the most problematic. At least my testing at home says so, so it’s ready for a review.

#16 Updated by intrigeri 2016-07-13 13:52:00

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

What’s the value of checking for a (limited list) of error conditions like SSHFailed.png, SSHError.png and SSHDenied.png? (I’m asking the person who is proposing this branch for merging, not the one who wrote that code initially.)

Same question for commit:920baa7. You wrote “We’ll get better output about the reason why the step failed”, but then how is that output useful? It seems to me that it adds basically nothing compared to looking at the failure screenshot, while it gives us more code and images to maintain.

The wording change brought by commit:273dbe2 makes the text less correct: a SFTP server has no fingerprint; it has a key, that itself has a fingerprint. I advice simply reverting to the previous wording, that feels more correct technically, and was written with someone who’s better at English than you or I.

Here also, please drop the useless and racy if $vm.has_process? test when killing processes.

I have concerns about commit:f4e064f: clicking the close button doesn’t guarantee that Files has been closed successfully on next try, so I don’t think that this change reliably fixes the problem it tries to address. Of course I totally trust that it works for you, and this change likely won’t make things any worse, but once it’s deployed on dozens of branches, we might realize that you were right in identifying a problem, but that the fix is insufficient. What do you think?

Great work!

#17 Updated by bertagaz 2016-07-15 04:49:25

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

intrigeri wrote:
> What’s the value of checking for a (limited list) of error conditions like SSHFailed.png, SSHError.png and SSHDenied.png? (I’m asking the person who is proposing this branch for merging, not the one who wrote that code initially.)
>
> Same question for commit:920baa7. You wrote “We’ll get better output about the reason why the step failed”, but then how is that output useful? It seems to me that it adds basically nothing compared to looking at the failure screenshot, while it gives us more code and images to maintain.

You make a point. Removed them with commit:4e5e1fa and commit:a2cf03a

> The wording change brought by commit:273dbe2 makes the text less correct: a SFTP server has no fingerprint; it has a key, that itself has a fingerprint. I advice simply reverting to the previous wording, that feels more correct technically, and was written with someone who’s better at English than you or I.

Ok, commit:9a9ad14

> Here also, please drop the useless and racy if $vm.has_process? test when killing processes.

commit:de95a13

> I have concerns about commit:f4e064f: clicking the close button doesn’t guarantee that Files has been closed successfully on next try, so I don’t think that this change reliably fixes the problem it tries to address. Of course I totally trust that it works for you, and this change likely won’t make things any worse, but once it’s deployed on dozens of branches, we might realize that you were right in identifying a problem, but that the fix is insufficient. What do you think?

Good point. Would commit:36a2d8e satisfy you?

#18 Updated by intrigeri 2016-07-15 08:34:40

• Assignee changed from intrigeri to anonym

>> I have concerns about commit:f4e064f: clicking the close button doesn’t guarantee that Files has been closed successfully on next try, so I don’t think that this change reliably fixes the problem it tries to address. Of course I totally trust that it works for you, and this change likely won’t make things any worse, but once it’s deployed on dozens of branches, we might realize that you were right in identifying a problem, but that the fix is insufficient. What do you think?

> Good point. Would commit:36a2d8e satisfy you?

I guess waiting for something more specific to Files to disappear would be more robust / future-proof, but this seems good enough to me => code review passes for me => assigning to anonym for the last review’n’merge steps :)

#19 Updated by intrigeri 2016-07-21 06:02:30

• Assignee changed from anonym to intrigeri

#20 Updated by intrigeri 2016-07-21 08:58:33

• 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

Merged, thanks!

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

• Status changed from Fix committed to Resolved