Closed Bug 1029363 Opened 7 years ago Closed 7 years ago

browser_translation_fhr.js fails if it runs on its own

Categories

(Firefox :: Translation, defect)

x86
macOS
defect
Not set
normal
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 33
Iteration:
33.3
Tracking Status
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: mano, Assigned: mano)

Details

Attachments

(1 file)

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.
Attached patch patch.diffSplinter Review
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #8444993 - Flags: review?(ttaubert)
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+
https://hg.mozilla.org/mozilla-central/rev/1e7689d04da4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
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?
Attachment #8444993 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Iteration: --- → 33.2
Points: --- → 2
Flags: firefox-backlog+
Iteration: 33.2 → 33.3
QA Whiteboard: [qa?]
Hi Mano, can you mark this bug as [qa+] or [qa-] for verification.
Flags: needinfo?(mano)
QA Whiteboard: [qa?] → [qa-]
Flags: needinfo?(mano)
You need to log in before you can comment on or make changes to this bug.