Closed
Bug 1029363
Opened 10 years ago
Closed 10 years ago
browser_translation_fhr.js fails if it runs on its own
Categories
(Firefox :: Translation, defect)
Tracking
()
People
(Reporter: asaf, Assigned: asaf)
Details
Attachments
(1 file)
(deleted),
patch
|
ttaubert
:
review+
Gavin
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In https://hg.mozilla.org/integration/fx-team/rev/5e0288a4e398 I changed browser_translation_fhr.js to check the _additions_ to each metrics after each test. At first, I set the initial values to compare with to zeros. That worked when I ran the test on its own, but obviously failed when brosser_translation_infobar.js ran first. So, I removed the initial (zero) values, and just called updateMetrics in the setup method. That fixed the multiple tests case, in exchange for breaking the sole test one, because updateMetrics fails to find any initial values. And if it fails, it think it's because it's midnight ("Getting metrics around midnight may fail sometimes").
A "proper" solution is an overkill, I think. So I'm going to fix this by running some translation in the setup method. That way, the test will always run as if it's running after some other translation test did, leaving some metrics in place.
Assignee | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
Comment on attachment 8444993 [details] [diff] [review]
patch.diff
Review of attachment 8444993 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the bug reference removed. Thanks!
::: browser/components/translation/test/browser_translation_fhr.js
@@ +71,5 @@
> Services.prefs.clearUserPref("browser.translation.ui.show");
> });
>
> + // Bug 1029363 - make sure there are some initial metrics in place when
> + // the test starts.
I don't think we should reference the bug here, that just makes it seem like there's a remaining issue.
Attachment #8444993 -
Flags: review?(ttaubert) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Comment 5•10 years ago
|
||
Comment on attachment 8444993 [details] [diff] [review]
patch.diff
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Translation feature
User impact if declined: required to land bug 977774 (see bug 977774 comment 24(
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): small
String or IDL/UUID changes made by this patch: none
Attachment #8444993 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8444993 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 6•10 years ago
|
||
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
Updated•10 years ago
|
Iteration: --- → 33.2
Points: --- → 2
Flags: firefox-backlog+
Updated•10 years ago
|
Iteration: 33.2 → 33.3
QA Whiteboard: [qa?]
Comment 7•10 years ago
|
||
Hi Mano, can you mark this bug as [qa+] or [qa-] for verification.
Flags: needinfo?(mano)
Assignee | ||
Updated•10 years ago
|
QA Whiteboard: [qa?] → [qa-]
Flags: needinfo?(mano)
You need to log in
before you can comment on or make changes to this bug.
Description
•