Project

General

Profile

Feature #5632

Feature #5330: Test suite: identify and document race conditions

Test suite: more robust encryption feature

Added by Tails about 6 years ago. Updated over 4 years ago.

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

100%

Feature Branch:
test/5632-hopefully-more-robust-encryption
Type of work:
Code
Blueprint:
Starter:
No
Affected tool:

Description

All scenarios in features/encryption.feature failed on a slow system, due to the "sleep" time between "clicking the gpgapplet icon" and "sending a keypress that was supposed to be received by the gpgapplet contextual menu" was too small.

This sleep time was bumped in commit 4eff913, but really, such code is too racy:

@screen.click("GpgAppletIconNormal.png")
sleep 2
@screen.type("k")

It would be good to implement something better, like... waiting for the context menu to be displayed before sending the "k" keypress.


Related issues

Related to Tails - Bug #9258: Accessing systray icons in the test suite is fragile Resolved 04/18/2015

Associated revisions

Revision bf1c04ea
Added by Tails developers over 4 years ago

Merge remote-tracking branch 'origin/test/5632-hopefully-more-robust-encryption' into testing

Fix-committed: #5632

History

#1 Updated by intrigeri almost 6 years ago

  • Category set to Test suite
  • Starter set to No

#2 Updated by intrigeri over 5 years ago

  • Parent task set to #5330

#3 Updated by BitingBird over 5 years ago

  • Subject changed from test suite: more robust encryption feature to Test suite: more robust encryption feature

#4 Updated by intrigeri almost 5 years ago

I've seen it fail with "The clipboard does not contain valid input data", presumably caused by this race condition or another.

#6 Updated by anonym over 4 years ago

  • Target version set to Tails_1.5

#7 Updated by anonym over 4 years ago

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

#8 Updated by anonym over 4 years ago

  • Assignee set to kytv

#9 Updated by intrigeri over 4 years ago

intrigeri wrote:

I've seen it fail with "The clipboard does not contain valid input data", presumably caused by this race condition or another.

Reproduced on isotester1.lizard.

#10 Updated by kytv over 4 years ago

  • Status changed from Confirmed to In Progress
  • Assignee changed from kytv to anonym
  • Target version changed from Tails_1.4 to Tails_1.3.2
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to kytv:bugfix/5632-hopefully-more-robust-encryption

I've run this feature successfully 15/15 times with these changes.

#11 Updated by kytv over 4 years ago

  • Assignee changed from anonym to kytv
  • % Done changed from 50 to 30
  • QA Check deleted (Ready for QA)

Perhaps I should continue testing/refining.

#12 Updated by kytv over 4 years ago

  • Feature Branch deleted (kytv:bugfix/5632-hopefully-more-robust-encryption)

#13 Updated by kytv over 4 years ago

  • Assignee changed from kytv to anonym
  • Target version changed from Tails_1.3.2 to Tails_1.3
  • % Done changed from 30 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to kytv:test/5632-hopefully-more-robust-encryption

The changes in this branch have made this feature far more reliable for me, with my only failures--so far--being the transient failure finding the GnomeApplicationsTails.png image.

#14 Updated by anonym over 4 years ago

  • Assignee changed from anonym to kytv
  • % Done changed from 50 to 70
  • Feature Branch changed from kytv:test/5632-hopefully-more-robust-encryption to test/5632-hopefully-more-robust-encryption

The branch looks good, and even though I very rarely experienced problems with this feature, it certainly looks like it didn't make it worse, especially by removing all those static sleep:s. Also, the killing of duplicated code alone is welcome.

While I could merge this branch as-is, I noticed something unrelated to your changes which possibly could yield some robustness improvements that I think should be fixed first: In the "I sign the message using my OpenPGP key" and "I both encrypt and sign the message using my OpenPGP key" steps we use encrypt_sign_helper(), and since we're gonna sign we'll be asked for our private key's passphrase. Now, encrypt_sign_helper() calls maybe_deal_with_pinentry() to deal with that, but in the code blocks we send to encrypt_sign_helper() in these two steps we also wait for PinEntryPrompt.png and enter the passphrase, so it's done twice. I think the worst thing this can result in is that the passphrase is typed a second time, now into Gedit, which shouldn't cause any problem given how things work now (but who knows?), but it's incorrect nevertheless and could be problematic for some future usage of these steps, so let's fix this now. See commit 3d31c12.

Since I already was digging into this branch, I also pushed commit a9eb1db which I just think is good style, although not necessary.

If you ACK these changes, kytv, I'll merge the branch.

#15 Updated by kytv over 4 years ago

  • Assignee changed from kytv to anonym
  • % Done changed from 70 to 90
  • QA Check changed from Ready for QA to Pass
  • Feature Branch changed from test/5632-hopefully-more-robust-encryption to test/5632-hopefully-more-robust-encryptio

anonym wrote:

The branch looks good, and even though I very rarely experienced problems with this feature, it certainly looks like it didn't make it worse, especially by removing all those static sleep:s. Also, the killing of duplicated code alone is welcome.

Woot

See commit 3d31c12.

Looks good.

Since I already was digging into this branch, I also pushed commit a9eb1db which I just think is good style, although not necessary.

Good change too.

If you ACK these changes, kytv, I'll merge the branch.

Full ACK.

#16 Updated by Tails over 4 years ago

  • Status changed from In Progress to Fix committed
  • % Done changed from 90 to 100

Applied in changeset commit:81c1a096748646f9066ed788562df9bc2d6d210c.

#17 Updated by anonym over 4 years ago

  • Assignee deleted (anonym)
  • Feature Branch changed from test/5632-hopefully-more-robust-encryptio to test/5632-hopefully-more-robust-encryption

Fixed the branch name in the last minute.

#18 Updated by BitingBird over 4 years ago

  • Status changed from Fix committed to Resolved

#19 Updated by kytv over 4 years ago

  • Related to Bug #9258: Accessing systray icons in the test suite is fragile added

Also available in: Atom PDF