Closed Bug 1220246 Opened 4 years ago Closed 2 years ago

Intermittent test_geodefaults.js | should_get_geo_defaults_only_once - [should_get_geo_defaults_only_once : 104] "undefined" == "number"

Categories

(Firefox :: Search, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: KWierso, Assigned: florian)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Blocks: 1203167
The failure occurs when it takes more than 1000ms for the local test HTTP server to reply to the /lookup_defaults requests. The test slaves that showed this failure must be pretty overloaded. The attached patch forces a 1000ms wait to reproduce locally and debug the test.
Try looks green, so I would like to try landing this simple patch.

The problem here is that whenever something related to search is changed, the search service caches things to disk with a 1000ms delay. The test currently expects one notification of the file being written to disk, but when the HTTP server takes more than a second to respond, the file gets written to disk twice, and the promise only waits for the first notification. The patch addresses this by creating the promise after the search service is already done initializing. There's no risk of the (relevant) notification having been already fired at that point, because the file is written to disk with a delay.
Attachment #8684216 - Flags: review?(gijskruitbosch+bugs)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attached patch Patch v2Splinter Review
Looks like the reason why we are writing the file to disk twice during the should_get_geo_defaults_only_once test is that the promiseAfterCache from the no_request_if_prefed_off test can be resolved due to writing the initial cache rather than due to the currentEngine/resetToOriginalDefaultEngine code block. And also, this code is pointless now that the metadata is no longer stored in a separate file.
Attachment #8684224 - Flags: review?(gijskruitbosch+bugs)
Attachment #8684216 - Attachment is obsolete: true
Attachment #8684216 - Flags: review?(gijskruitbosch+bugs)
Attachment #8684224 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/c921afa9fc3e164ab52dc4564349a92e16663317
Bug 1220246 - fix test_geodefaults.js to avoid failing when the http server takes more than 1000ms to reply, r=Gijs.
https://hg.mozilla.org/mozilla-central/rev/c921afa9fc3e
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Looks like this never really went away.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Status: REOPENED → RESOLVED
Closed: 4 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.