Feature #5330

Test suite: identify and document race conditions

Added by Tails 2013-07-18 07:39:10 . Updated 2016-12-10 17:11:42 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Tails_2.9.1
Start date:
2015-01-19
Due date:
% Done:

100%

Feature Branch:
kytv/test/5330-race-conditions
Type of work:
Research
Blueprint:

Starter:
0
Affected tool:
Deliverable for:

Description

Updated scope of this ticket: Feature #5330#note-23.

A sleep that’s good enough on your hardware might not be good enough on mine (or in a nested VM setup in particular). We need to, whenever possible, use reliable ways to detect the state we are waiting for instead of sleep:ing some fixed time and hoping we’ve reached that state by then.

Just run git grep sleep features/step_definitions/*.rb for an up-to-date list of these "violations".

Subtasks

Feature #5632: Test suite: more robust encryption feature Resolved

100
Bug #8742: Synaptic tests frequently fail Resolved

100
Bug #9258: Accessing systray icons in the test suite is fragile Resolved

100
Feature #9877: Unused @gksu@ step in test suite Resolved

100

Related issues

Related to Tails - Bug #10222: Pidgin tests with certificates are fragile Resolved 2015-09-19
Related to Tails - Bug #9330: Focusing windows during the test suite is fragile Resolved 2015-09-19

History

#1 Updated by intrigeri 2013-10-04 09:07:41

  • Category set to Test suite
  • Starter set to No

#2 Updated by BitingBird 2014-06-09 10:23:31

  • Subject changed from test suite: fix race conditions to Test suite: fix race conditions

#3 Updated by intrigeri 2014-10-16 03:14:44

  • blocks Feature #5288: Run the test suite automatically on autobuilt ISOs added

#4 Updated by intrigeri 2015-01-06 12:53:33

  • blocked by deleted (Feature #5288: Run the test suite automatically on autobuilt ISOs)

#5 Updated by intrigeri 2015-01-06 12:53:43

#6 Updated by sajolida 2015-01-07 12:37:12

  • blocks #8538 added

#7 Updated by anonym 2015-01-09 14:45:05

  • Target version set to Tails_1.5

#8 Updated by anonym 2015-01-10 17:41:50

  • Assignee set to kytv
  • Target version changed from Tails_1.5 to Tails_1.4

#9 Updated by intrigeri 2015-05-09 02:49:54

  • Target version changed from Tails_1.4 to Tails_1.4.1

Postponing, but I believe some progress on it was incidentally made here and there, while working on other aspects of Feature #8539 => it’s less of a daunting task than it used to be :)

#10 Updated by intrigeri 2015-06-29 00:50:14

  • Target version changed from Tails_1.4.1 to Tails_1.5

#11 Updated by intrigeri 2015-07-19 07:36:22

  • Status changed from Confirmed to In Progress

#12 Updated by kytv 2015-08-11 10:35:34

  • Target version changed from Tails_1.5 to Tails_1.6

#13 Updated by kytv 2015-09-20 15:36:34

  • related to Bug #10222: Pidgin tests with certificates are fragile added

#14 Updated by kytv 2015-09-20 15:36:56

  • related to Bug #9330: Focusing windows during the test suite is fragile added

#15 Updated by kytv 2015-09-20 15:38:35

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

Progress has been made but there’s still more work to be done here.

#16 Updated by intrigeri 2015-09-21 02:31:52

kytv wrote:
> Progress has been made but there’s still more work to be done here.

Indeed, and while reviewing the 1.5.1..1.6 diff I noticed that we just… added a sleep 1. I suggest we stop adding such things, otherwise the task of removing them will be never ending :)

#17 Updated by kytv 2015-09-26 08:30:53

It’s been clarified that this can also be solved by

  • documenting why some sleep:s are necessary (such as there aren’t usable visual indicators on the screen); or
  • bumping the sleep:s for tests which may be long running, such as for tests which may take 5 minutes, maybe a sleep of 1 should be bumped to 5 or maybe 10.

#18 Updated by kytv 2015-11-04 10:46:04

  • Target version changed from Tails_1.7 to Tails_1.8

#19 Updated by intrigeri 2015-11-06 05:27:09

I’d like to see a realistic target version set on this one, instead of semi-automatic postponing. Perhaps keep in on the side for now, and focus on it in December? Of course it depends on your other commitments / deadlines.

#20 Updated by kytv 2015-11-28 18:26:08

  • Assignee changed from kytv to anonym
  • QA Check set to Ready for QA
  • Feature Branch set to kytv/test/5330-race-conditions

This should hopefully address the issues discussed with intrigeri.

#21 Updated by anonym 2015-12-16 14:06:06

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

#22 Updated by anonym 2016-01-19 15:25:34

  • Assignee changed from anonym to kytv
  • QA Check changed from Ready for QA to Dev Needed
  • Type of work changed from Code to Research

I’ve merged kytv/test/5330-race-conditions but only because we desperately need robustness. IMHO this ticket is about getting rid of sleep:s, because they are inherently racy. In the comments you add you say “we have no reliable visual indicators that we can watch to know when typing is ‘safe’”, which I suppose is true, but we need something else then. Perhaps Feature #10721 or something (i.e. a way to query Xorg what type of element it has focused? surely that is possible since e.g. Florence can do it)?

#23 Updated by intrigeri 2016-01-19 16:03:33

> IMHO this ticket is about getting rid of sleep:s, because they are inherently racy.

It originally was but then it was re-focussed (Feature #5330#note-17) because the original scope was a bit too broad and scary; so kytv did just that. Granted, the ticket description should have been updated back then. I think we should really use Feature #5330#note-17 as the goal of this ticket, so we can close it soon, and file a ticket under Bug #10288 for each instance of sleep that we think is problematic (possibly all of them), so we can fix them once at a time (except that when they are similar enough, of course a less fine-grained approach can be more adequate).

#24 Updated by anonym 2016-02-05 20:29:19

  • Status changed from In Progress to Resolved
  • Assignee deleted (kytv)
  • QA Check changed from Dev Needed to Pass

intrigeri wrote:
> > IMHO this ticket is about getting rid of sleep:s, because they are inherently racy.
>
> It originally was but then it was re-focussed (Feature #5330#note-17) because the original scope was a bit too broad and scary; so kytv did just that. Granted, the ticket description should have been updated back then. I think we should really use Feature #5330#note-17 as the goal of this ticket, so we can close it soon, and file a ticket under Bug #10288 for each instance of sleep that we think is problematic (possibly all of them), so we can fix them once at a time (except that when they are similar enough, of course a less fine-grained approach can be more adequate).

Ack => closing.

#25 Updated by anonym 2016-02-05 20:32:39

  • Status changed from Resolved to In Progress
  • Assignee set to kytv
  • Target version changed from Tails_2.0 to Tails_2.3
  • QA Check changed from Pass to Dev Needed

anonym wrote:
> Ack => closing.

Well, actually we need to file a ticket for each sleep under Bug #10288 first. Postponing, however.

#26 Updated by intrigeri 2016-03-03 16:21:09

  • Assignee changed from kytv to intrigeri

I’ll create the tickets.

#27 Updated by anonym 2016-03-03 16:58:37

  • Assignee changed from intrigeri to anonym
  • Target version changed from Tails_2.3 to Tails_2.4

I’ll take over this one.

#28 Updated by anonym 2016-06-08 01:34:56

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

#29 Updated by sajolida 2016-07-14 00:33:53

Does it still makes sense to have this blocking #8538?

#30 Updated by intrigeri 2016-07-16 05:22:21

> Does it still makes sense to have this blocking #8538?

That’s our only way to keep track of how we’re doing on tickets that were part of #8538, so I’d say yes. I see no reason to hide the fact that this one is late (and it’s no big deal, really).

#31 Updated by intrigeri 2016-07-21 06:23:44

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

#32 Updated by intrigeri 2016-07-21 06:25:00

  • Subject changed from Test suite: fix race conditions to Test suite: identify and document race conditions
  • Description updated

#33 Updated by bertagaz 2016-11-17 17:38:34

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

#34 Updated by anonym 2016-12-05 20:40:02

New goal of this ticket: go through the existing instances of sleep() and add comments justifying why they are needed and won’t break on different hardware. And maybe find instances we can kill along the way.

#35 Updated by anonym 2016-12-10 17:11:42

  • Status changed from In Progress to Resolved
  • Assignee deleted (anonym)
  • QA Check changed from Dev Needed to Pass

anonym wrote:
> New goal of this ticket: go through the existing instances of sleep() and add comments justifying why they are needed and won’t break on different hardware. And maybe find instances we can kill along the way.

Apparently we justify all sleep:s that could be racy vs Sikuli and similar. The rest are used for e.g. rate limiting loops and similar ok stuff. So, we are done! Yay!