Bug #9286
Write regression test for the Tor Browser's New identity feature
100%
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!
Subtasks
History
#1 Updated by anonym 2015-04-28 09:15:02
- blocks #8538 added
#2 Updated by intrigeri 2015-04-28 16:28:30
- Parent task set to
Feature #8543
#3 Updated by kytv 2015-05-14 18:05:11
- 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 2015-05-26 11:17:51
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 2015-05-26 11:18:08
- Assignee changed from anonym to kytv
- QA Check changed from Info Needed to Dev Needed
#7 Updated by kytv 2015-05-27 14:37:46
- Status changed from Confirmed to In Progress
#8 Updated by kytv 2015-05-27 14:38:18
- % Done changed from 0 to 20
Done, now at the test testing stage.
#9 Updated by kytv 2015-05-27 22:35:23
- 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 2015-05-28 13:51:22
- 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 2015-05-28 14:30:26
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 2015-05-28 16:57:29
- 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 2015-06-02 11:48:40
Applied in changeset commit:d9dcc249fa6e7601b385e85bb00b8d31c91eef8d.
#14 Updated by anonym 2015-06-02 11:48:42
- Status changed from In Progress to Fix committed
- % Done changed from 20 to 100
Applied in changeset commit:d0241f7753b942d77a5be5d8613ce9d59e22289e.
#15 Updated by anonym 2015-06-02 12:05:48
- Assignee deleted (
anonym) - QA Check changed from Ready for QA to Pass
#16 Updated by intrigeri 2015-07-03 03:41:25
- Status changed from Fix committed to Resolved