Closed Bug 1612531 Opened 4 years ago Closed 3 years ago

test_TelemetryEnvironment.js doesn't run under the debugger and is too big to easily debug sections of it

Categories

(Toolkit :: Telemetry, task, P2)

task
Points:
3

Tracking

()

RESOLVED FIXED
88 Branch
Iteration:
88.2 - Mar 8 - Mar 21
Tracking Status
firefox88 --- fixed

People

(Reporter: standard8, Assigned: standard8)

Details

Attachments

(1 file)

I'm trying to work on some of the search engine telemetry reports. Attempting to run the test within the debugger fails, pretty much in whichever test I try to run it in.

Unfortunately, the size and numbers of the tests within the file makes it difficult to skip individual tests.

This is the first issue I get when running the test under the debugger. This doesn't happen when it is run on its own:

$ ./mach xpcshell-test toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
...
toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
  FAIL test_checkEnvironment - [test_checkEnvironment : 1107] addons database is not loaded - true == false
/Users/mark/dev/gecko/objdir-ff-opt/_tests/xpcshell/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:test_checkEnvironment:1107
/Users/mark/dev/gecko/testing/xpcshell/head.js:run_next_test/_run_next_test/<:1567
/Users/mark/dev/gecko/testing/xpcshell/head.js:_run_next_test:1567
/Users/mark/dev/gecko/testing/xpcshell/head.js:run:735
/Users/mark/dev/gecko/testing/xpcshell/head.js:_do_main:246
/Users/mark/dev/gecko/testing/xpcshell/head.js:_execute_test:573
-e:null:1
  FAIL toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js - xpcshell return code: 0
ERROR Unexpected exception NS_ERROR_ABORT: 
_abort_failed_test@/Users/mark/dev/gecko/testing/xpcshell/head.js:791:20
do_report_result@/Users/mark/dev/gecko/testing/xpcshell/head.js:892:5
Assert<@/Users/mark/dev/gecko/testing/xpcshell/head.js:67:21
proto.report@resource://testing-common/Assert.jsm:233:10
equal@resource://testing-common/Assert.jsm:275:8
test_checkEnvironment@/Users/mark/dev/gecko/objdir-ff-opt/_tests/xpcshell/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:1107:10
run_next_test/_run_next_test/<@/Users/mark/dev/gecko/testing/xpcshell/head.js:1567:22
_run_next_test@/Users/mark/dev/gecko/testing/xpcshell/head.js:1567:38
run@/Users/mark/dev/gecko/testing/xpcshell/head.js:735:9
_do_main@/Users/mark/dev/gecko/testing/xpcshell/head.js:246:6
_execute_test@/Users/mark/dev/gecko/testing/xpcshell/head.js:573:5
@-e:1:1

The Telemetry Environment is a large, poorly-understood and deeply-depended-upon piece of data collection that has many mysterious owners and behaviours. Unfortunately the tests also reflect this.

My guess (and guess it is) is that the debugger fiddles with the delicate startup timing machinery that underpins so much of the Environment's complexity. Thus it allows the addon db to fully load before we're ready for it in the test. (which makes it a bad test)

I'm not exactly sure how best to help you out here. Did you have a particular solution in mind?

Flags: needinfo?(standard8)

I think generally it would be nice if:

  • The test was split up into multiple tests.
  • The test runs under the debugger.

Both of these would help when trying to debug it, though I understand that they're possibly intertwined and maybe complicated due to the existing code.

Flags: needinfo?(standard8)

The priority flag is not set for this bug.
:chutten, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(chutten)

Thanks, that's very helpful and I agree. It seems like breaking this into chunks might get us most of the value for less of the pain (and be a step in the direction of being able to run under the debugger), so I'll treat this bug as being about that for now.

Unfortunately I of course don't have time in the next little while to look into this, so into the hopeful land of P2 it goes for now.

Type: defect → task
Points: --- → 3
Flags: needinfo?(chutten)
Priority: -- → P2
Assignee: nobody → standard8
Status: NEW → ASSIGNED

I've only split the search tests out here, as they are the ones that concern me the most, and the existing set-up was making it difficult to do the changes in bug 1687932 due to how the add-on manager and search service are set up.

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6bbff7fcde3
Split out search telemetry environment tests and remove use of nsISearchEngine.addEngineWithDetails. r=chutten
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
Iteration: --- → 88.2 - Mar 8 - Mar 21
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: