Tails

#3 Updated by kytv 2015-01-19 22:57:43

• Assignee changed from kytv to anonym
• % Done changed from 20 to 40
• QA Check set to Ready for QA
• Feature Branch set to kytv:bugfix/8742-synaptic-test-robustness

#4 Updated by intrigeri 2015-01-21 10:00:44

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

Your branch bundles a commit from kytv:test/8697-update-apt-get, but its history was rewritten so it has a different ID. Please either rebase on top of devel, or rebase on top of kytv:test/8697-update-apt-get and mark this ticket as blocked by Bug #8697. Thanks!

#5 Updated by kytv 2015-01-21 12:36:04

• QA Check changed from Dev Needed to Ready for QA

Oops. Rebased on devel. Sorry about that.

#6 Updated by kytv 2015-01-21 13:17:25

• Assignee changed from kytv to anonym

#7 Updated by anonym 2015-01-23 12:01:22

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

kytv wrote:
> Oops. Rebased on devel. Sorry about that.

Hmm, after fetching your repo git log --oneline origin/devel..kytv/bugfix/8742-synaptic-test-robustness still lists:

3618e8e Ensure that the test will fail if "apt-get X" commands fail.

which seems to be an other version of de8959c, which now is in devel after I merged kytv:test/8697-update-apt-get. Please drop 3618e8e and force push.

Here’s a review of what’s currently commit d6ff7d0 (“Reduce the number of static sleeps.”):

> + @screen.wait(‘SynapticPackageList.png’, 30)

You forgot to git add features/images/SynapticPackageList.png. :)

> @ # We do this after a Reload, so the interface will be frozen until@
> @ # the package list has been loaded@
> - try_for(60, :msg => "Failed to open the Synaptic 'Find' window") {
> - screen.type(“f”, Sikuli::KeyModifier.CTRL) # Find key@
> - screen.find(‘SynapticSearch.png’)@
> - }
> + screen.type(“f”, Sikuli::KeyModifier.CTRL) # Find key@
> + screen.wait_and_click(‘SynapticSearch.png’, 10)@

That try_for block has proven essential before since the GUI is unresponsive, but I assume that the point of waiting for SynapticPackageList.png in the previous step is to ensure that the GUI is responsive. If so, great! But please remove the comment, and possibly add an appropriate comment above screen.wait('SynapticPackageList.png', 30).

> + screen.wait('SynapticCowsaySearchResultSelected.png', 20) > [...] > + screen.wait(‘SynapticCowsayMarked.png’, 10)
> + @screen.wait_and_click(‘SynapticApply.png’, 10)

You forgot to @git add" these images too. :)

> - screen.type("p", Sikuli::KeyModifier.CTRL) # Apply > [...] > + #screen.type(“p”, Sikuli::KeyModifier.CTRL) # Apply

Don’t comment code; kill it! :)

> + screen.wait_and_click('SynapticPolicyKitAuthPrompt.png', 30) > deal_with_polkit_prompt('SynapticPolicyKitAuthPrompt.png', sudo_password)

If the _click indeed is necessary I recommend moving the wait_and_click into the deal_with_polkit_prompt helper (located in features/step_definitions/common_steps.rb). Here’s how it looks:

> def deal_with_polkit_prompt (image, password)
> @screen.wait(image, 60)
> […]

So replace that wait with wait_and_click.

I’d also like to raise an interesting issue here: in the current state of the code, the wait_and_click you added would move the mouse cursor on top of the image we later wait for on in deal_with_polkit_prompt, hence potentially blocking it from being found by Sikuli. Sure, we use unclutter to try to hide the cursor, but it is pretty unreliable, and we should really try to explicitly avoid situations like this when possible. Yes, Tails’ automated test suite is an interesting place… :)

However, it may be that the _click isn’t needed. I have a theory; let’s look a bit more on deal_with_polkit_prompt:

> def deal_with_polkit_prompt (image, password)
> screen.wait(image, 60) > sleep 1 # wait for weird fade-in to unblock the "Ok" button > screen.type(password)
> […]

Perhaps what you really is hit by is that the sleep 1 isn’t enough for you? Actually it’s unclear to me what your exact issue with this test is, but I suggest you experiment a bit more with the hypothesis that it’s this static sleep that’s the culprit. Perhaps the situation can be improved by wait:ing on the OK button to unfreeze, or similar?

#8 Updated by intrigeri 2015-02-06 17:48:43

• blocks #8538 added

#11 Updated by kytv 2015-02-08 12:01:21

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

Rebased on devel and force pushed.

#13 Updated by anonym 2015-02-11 10:36:43

• Assignee changed from anonym to kytv
• QA Check changed from Ready for QA to Info Needed

> + #screen.wait_and_click(‘SynapticPolicyKitAuthPrompt.png’, 30)@

Again, let’s not add any commented out code.

> - sleep 1 # wait for weird fade-in to unblock the "Ok" button
> + sleep 3 # wait for weird fade-in to unblock the "Ok" button

I’m surprised this is needed. I actually had a closer look, and the “weird fade-in” isn’t even happening in Wheezy, but was something Squeeze-specific. I’d expect us to be able to remove the sleep, but obviously you experience some sort of issue here. Can you please describe what the issue is more precisely?

#14 Updated by kytv 2015-02-15 18:17:07

• Target version changed from Tails_1.3 to Tails_1.3.2

#15 Updated by kytv 2015-02-15 20:15:08

• Target version changed from Tails_1.3.2 to Tails_1.3

#16 Updated by kytv 2015-02-15 23:13:44

• Assignee changed from kytv to anonym
• QA Check changed from Info Needed to Ready for QA

anonym wrote:

> I’m surprised this is needed.

You were absolutely right. I removed the sleep and successfully re-ran this test multiple times without a hiccup. I don’t recall what I had seen before, but I’m certainly not seeing it now.

I merged in the testing branch, removed the sleep statement and the commented out line which was inadvertently left in.

#17 Updated by Tails 2015-02-18 09:03:29

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

Applied in changeset commit:c9aeac15bd4da41ce79fc7c073fd0ff22cd3940e.

#18 Updated by anonym 2015-02-18 09:03:56

• Assignee deleted (anonym)
• QA Check changed from Ready for QA to Pass

Merged!

#19 Updated by BitingBird 2015-02-24 22:42:57

• Status changed from Fix committed to Resolved