Project

General

Profile

Feature #5330

Test suite: identify and document race conditions

Added by Tails over 5 years ago. Updated almost 2 years ago.

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

100%

QA Check:
Pass
Feature Branch:
kytv/test/5330-race-conditions
Type of work:
Research
Blueprint:
Starter:
No
Affected tool:

Description

Updated scope of this ticket: #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 featureResolved

Bug #8742: Synaptic tests frequently failResolved

Bug #9258: Accessing systray icons in the test suite is fragileResolved

Feature #9877: Unused @gksu@ step in test suiteResolved


Related issues

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

Associated revisions

Revision c2abc706 (diff)
Added by kytv almost 3 years ago

Bump necessary static sleeps and document why they're needed

Will-fix: #5330

History

#1 Updated by intrigeri about 5 years ago

  • Category set to Test suite
  • Starter set to No

#2 Updated by BitingBird over 4 years ago

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

#3 Updated by intrigeri about 4 years ago

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

#4 Updated by intrigeri almost 4 years ago

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

#5 Updated by intrigeri almost 4 years ago

  • Parent task set to #8539

#7 Updated by anonym almost 4 years ago

  • Target version set to Tails_1.5

#8 Updated by anonym almost 4 years ago

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

#9 Updated by intrigeri over 3 years ago

  • 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 #8539 => it's less of a daunting task than it used to be :)

#10 Updated by intrigeri over 3 years ago

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

#11 Updated by intrigeri over 3 years ago

  • Status changed from Confirmed to In Progress

#12 Updated by kytv about 3 years ago

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

#13 Updated by kytv about 3 years ago

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

#14 Updated by kytv about 3 years ago

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

#15 Updated by kytv about 3 years ago

  • 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 about 3 years ago

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 about 3 years ago

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 almost 3 years ago

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

#19 Updated by intrigeri almost 3 years ago

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 almost 3 years ago

  • 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 almost 3 years ago

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

#22 Updated by anonym almost 3 years ago

  • 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 #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 almost 3 years ago

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

It originally was but then it was re-focussed (#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 #5330#note-17 as the goal of this ticket, so we can close it soon, and file a ticket under #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 over 2 years ago

  • 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 (#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 #5330#note-17 as the goal of this ticket, so we can close it soon, and file a ticket under #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 over 2 years ago

  • 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 #10288 first. Postponing, however.

#26 Updated by intrigeri over 2 years ago

  • Assignee changed from kytv to intrigeri

I'll create the tickets.

#27 Updated by anonym over 2 years ago

  • 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 over 2 years ago

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

#29 Updated by sajolida over 2 years ago

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

#30 Updated by intrigeri over 2 years ago

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 over 2 years ago

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

#32 Updated by intrigeri over 2 years ago

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

#33 Updated by bertagaz almost 2 years ago

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

#34 Updated by anonym almost 2 years ago

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 almost 2 years ago

  • 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!

Also available in: Atom PDF