Bug #8698

Step "I open the address "([^"]*)" in the Tor Browser" is fragile

Added by intrigeri 2015-01-14 13:58:08 . Updated 2015-02-24 22:44:35 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Tails_1.3
Start date:
2015-01-14
Due date:
% Done:

100%

Feature Branch:
kytv:test/8698-open-address-tor-browser
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Browser
Deliverable for:

Description

  step "I open a new tab in the Tor Browser"
  @screen.click("TorBrowserAddressBar.png")

… assumes that the new tab is open immediately, as soon as one has clicked on the new tab button. I’ve seen it failed on isotester1.lizard. We should either replace click with wait_and_click (and e.g. a 5 seconds timeout), or , probably better, have the “I open a new tab in the Tor Browser” actually check that it has produced the intended result.

Subtasks

History

#1 Updated by intrigeri 2015-01-14 14:00:43

  • blocks #8538 added

#2 Updated by kytv 2015-01-16 12:23:12

  • Status changed from Confirmed to In Progress
  • Assignee set to kytv

#3 Updated by kytv 2015-01-16 13:33:45

  • Assignee changed from kytv to anonym
  • % Done changed from 0 to 30
  • QA Check set to Ready for QA
  • Feature Branch set to kytv:test/8698-open-address-tor-browser

The torified_browsing test passed 10/10 times.

#4 Updated by kytv 2015-01-16 15:21:39

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

#5 Updated by intrigeri 2015-01-21 10:17:27

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

I think that something like this would be nicer:

--- a/features/step_definitions/torified_browsing.rb
+++ b/features/step_definitions/torified_browsing.rb
@@ -1,12 +1,12 @@
 When /^I open a new tab in the Tor Browser$/ do
   next if @skip_steps_while_restoring_background
   @screen.click("TorBrowserNewTabButton.png")
+  @screen.wait("TorBrowserAddressBar.png", 10)
 end

 When /^I open the address "([^"]*)" in the Tor Browser$/ do |address|
   next if @skip_steps_while_restoring_background
   step "I open a new tab in the Tor Browser"
   @screen.click("TorBrowserAddressBar.png")
-  sleep 0.5
   @screen.type(address + Sikuli::Key.ENTER)
 end

… as the I open a new tab in the Tor Browser step would guarantee that, when it returns, a new tab was actually open.

Any reason not to go this way?

#6 Updated by anonym 2015-01-22 14:04:24

intrigeri wrote:
> I think that something like this would be nicer:
>
> […]
>
> … as the I open a new tab in the Tor Browser step would guarantee that, when it returns, a new tab was actually open.
>
> Any reason not to go this way?

I think what you propose is better according to the “probably better” hint in the description, which I agree with. kytv, feel free to rewrite the branch if you so prefer.

However, I see no reason why we can remove the static sleep. It was added in commit c37e3a8 because “[w]ithout this sleep time between ctrl+L and typing the URL, sometimes the first char of the URL is not typed in the URL bar”, which is a completely separate issue. I’d love to see it go, but I’d hate to re-introduce fragility in this fragility-fix. :) This is a general issue also present at other places (I remember there was something similar with the field in GNOME’s run dialog) and there’s no good way to detect whether a field has focus.

#7 Updated by kytv 2015-01-22 15:03:18

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

Yes, this was a better way of doing it.

Based on the note about rev c37e3a8 I re-added the static sleep, rewrote the branch, and force pushed.

#8 Updated by Tails 2015-01-23 13:16:27

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

Applied in changeset commit:4caf5e9ff877ce33d7ad0306a3f0b83e01380eed.

#9 Updated by anonym 2015-01-23 13:17:04

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

Merged!

#10 Updated by BitingBird 2015-02-24 22:44:35

  • Status changed from Fix committed to Resolved