Project

General

Profile

Bug #9286

Feature #8543: Write regression tests and tests for new features

Write regression test for the Tor Browser's New identity feature

Added by anonym over 4 years ago. Updated about 4 years ago.

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

100%

Feature Branch:
kytv:test/9286-new-identity
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

This one has broken several times, and surprised me very late when working on the migration to Tor Browser 4.5. Let's please automate it!

Associated revisions

Revision d9dcc249 (diff)
Added by kytv over 4 years ago

Automate testing Torbutton's 'New Identity' feature

Will-fix: #9286

Revision d0241f77
Added by anonym over 4 years ago

Merge remote-tracking branch 'kytv/test/9286-new-identity' into stable

Fix-committed: #9286

History

#2 Updated by intrigeri over 4 years ago

  • Parent task set to #8543

#3 Updated by kytv over 4 years ago

  • Assignee changed from kytv to anonym
  • QA Check set to Info Needed

Do you have any suggestions for testing this feature? I don't mean in Ruby but how would you suggest manually testing that it works? Would, e.g.,

1. open http://check.torproject.org
2. New Identity
3. (hopefully) see the Tails homepage load

be sufficient? Maybe looking for a new entry at /var/log/tor/log for New control connection opened?

#4 Updated by anonym over 4 years ago

kytv wrote:

Do you have any suggestions for testing this feature? I don't mean in Ruby but how would you suggest manually testing that it works? Would, e.g.,

1. open http://check.torproject.org
2. New Identity
3. (hopefully) see the Tails homepage load

This sounds sufficient to me.

be sufficient? Maybe looking for a new entry at /var/log/tor/log for New control connection opened?

This is overkill, and it feels that a simple implementation would be racy. Don't bother. Besides, at some point we may use the control socket instead.

IMHO the point (at least in this stage) is just to test that this feature appears to work, like 1, 2, 3 above will indicate. We only want to test for the regressions we've had before when this feature has completely broken and the window hasn't restarted and we've got error prompts and stuff like that.

#5 Updated by anonym over 4 years ago

  • Assignee changed from anonym to kytv
  • QA Check changed from Info Needed to Dev Needed

#6 Updated by intrigeri over 4 years ago

This sounds sufficient to me.

ACK!

#7 Updated by kytv over 4 years ago

  • Status changed from Confirmed to In Progress

#8 Updated by kytv over 4 years ago

  • % Done changed from 0 to 20

Done, now at the test testing stage.

#9 Updated by kytv over 4 years ago

  • Assignee changed from kytv to anonym
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to kytv:test/9286-new-identity

#10 Updated by anonym over 4 years ago

  • Assignee changed from anonym to kytv
  • QA Check changed from Ready for QA to Dev Needed
new file mode 120000
index 0000000..bff9844
--- /dev/null
+++ b/features/images/TorButtonNewIdentityConfirmation.png
@@ -0,0 +1 @@
+UnsafeBrowserStartVerification.png
\ No newline at end of file

I just have a question. We've been doing this image symlink dance for a while, but it's a bit ugly, and I wonder when it will cause trouble, e.g. a test is changed so an image is removed, only that some other image symlinked to it and this wasn't noticed. So, in this case "both" images is just the stock "GNOME Question diablog icon". So perhaps it should be renamed to GnomeQuestionDialogIcon.png? What do you think? If you agree, and do it, great! Otherwise I won't block on this now.

Any way, everything looks and works great! Nitpicking time!

-Given /^the Tor Browser has started and loaded the (startup page|Tails roadmap)$/ do |page|
+Given /^the Tor Browser (?:has started and )?load(?:ed|s)? the (startup page|Tails roadmap)$/ do |page|

This allows the gramatically incorrect step the Tor Browser load the ... when it should be "load*s*". You should remove the ? at the end of the (?:ed|s)? group.

Lastly, the new The Tor Browser's "New identity" feature works as expected scenario lacks the @check_tor_leaks tag. Actually, The Tor Browser directory is usable scenario lacks it too, so I wouldn't mind you adding it there at the same time (but in a separate commit).

#11 Updated by kytv over 4 years ago

anonym wrote:

So perhaps it should be renamed to GnomeQuestionDialogIcon.png? What do you think? If you agree, and do it, great! Otherwise I won't block on this now.

On it.

Any way, everything looks and works great! Nitpicking time!

On this too.

#12 Updated by kytv over 4 years ago

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

The image renaming and "nit picking" has been addressed and pushed.

#13 Updated by kytv over 4 years ago

#14 Updated by anonym over 4 years ago

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

#15 Updated by anonym over 4 years ago

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

#16 Updated by intrigeri about 4 years ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF