FOG User Testing Ergonomics - Design, Documentation, Implementation
Categories
(Toolkit :: Telemetry, task, P3)
Tracking
()
People
(Reporter: chutten, Unassigned)
References
(Depends on 6 open bugs)
Details
(Whiteboard: [telemetry:fog:m7])
FOG's users (data implementors) will need a way to easily write tests for their instrumentation code. We should design, implement, and document testing strategies for our users, tying together instrumentation with testing.
We might provide custom test utilities to check for instrumentation errors (invalid_state
and friends). We should provide examples. We must support more than just JavaScript test suites.
Metrics already have testGetValue
, this is for establishing the layer of norms above that.
Reporter | ||
Comment 1•4 years ago
|
||
A test ergo thing I noticed: Testing a Memory Distribution is kinda irritating because the buckets and sums returned in testGetValue
are in bytes, but the samples could be kilobytes or megabytes or whatever. Makes me wish for a units of measure system.
Reporter | ||
Comment 2•4 years ago
|
||
Another test ergo thing I noticed: test_get_num_errors requires that you know the errors that a metric is likely to have. Maybe we should throw an exception or something on test_get_value if there are errors?
And another: There's no way to clear data between test runs in xpcshell or GTest because the init happens when the suite starts. Maybe we can fix this in GTest with a custom fixture.
Reporter | ||
Comment 3•4 years ago
|
||
Yet another test ergo thing: for IPC tests we only have a JS API for awaiting the flush of all child data to the parent (await FOG.testFlushAllChildren()
). We should totally expose a similar method on Rust and C++.
Reporter | ||
Comment 4•4 years ago
|
||
And: The tests in t/c/g/xpcshell
cannot ever succeed in parallel because they both write metrics to the same profile simultaneously. We need to have a better xpcshell testing story here, perhaps using the new capability to specify a data path on FOG init to point each file to its own tempdir?
Reporter | ||
Comment 5•4 years ago
|
||
With a user/dev split in the Docs we could really benefit from user-facing Debugging information. A lot of it will be pointing folks at the Debug Ping Viewer, but we also have about:glean
that might make people happy. Also, though it might be a bit too much behind-the-curtain, a reference to how to turn on logging might be beneficial.
Reporter | ||
Comment 6•4 years ago
|
||
We should be specific that instrumentation is expected to be tested. No more testing-exception-unchanged!
Reporter | ||
Comment 7•4 years ago
|
||
We should link to the Testing Strategy document in the developer testing docs.
Reporter | ||
Comment 8•3 years ago
|
||
We may wish to replace the existing link on about:glean
or add a new one to our new user-centric documentation when it's ready.
Reporter | ||
Comment 9•3 years ago
|
||
User-facing test docs should include fundamentals on the test suites involved. Do we supply a gtest
test fixture to extend? Explain what that is and how to use it. Do we need xpcshell
tests to get a profile? Explain why and how that works.
Reporter | ||
Comment 10•3 years ago
|
||
TestResetFOG isn't implemented on Android, meaning any tests that need to clear state are gonna explode on Android. (cf the backout on bug 1751241)
This isn't ideal, as resetting Glean (via resetting FOG) is a central pillar to how we're going to make instrumentation tests more pleasant.
Jan-Erik: Can Glean be reset in Android from xpcshell, mochi, gtest, and friends? Was this NS_ERROR_NOT_IMPLEMENTED_DUE_TO_TIME
or NS_ERROR_NOT_IMPLEMENTED_BECAUSE_IMPOSSIBLE
?
Comment 11•3 years ago
|
||
I think it was a NS_ERROR_NOT_NEEDED_AT_THE_TIME
.
And my initial implementation didn't even have a functioning fog_init
, and so implementing fog_test_reset
was even less necessary.
Now that fog_init
does something in an Android build and is only used in tests we should be able to also provide fog_test_reset
there.
Reporter | ||
Comment 12•2 years ago
|
||
Would it be possible to ergonomically offer a timestamp-insensitive comparison operation for EventRecords returned via test_get_value
? I mean, we could certainly write one in Rust and expose it (or mirror it) in C++. But JS? Do we write this once per language, or add more to the FFI?
Or do we document clever ways to split the timestamp
off in the docs, like for JS:
let eventRecords = Glean.myCat.myEvent.testGetValue();
let {timestamp, ...record} = eventRecords[0];
Assert.deepEqual(record, {category: "my_cat", name: "my_event"});
Comment 13•2 years ago
|
||
via jlink on https://phabricator.services.mozilla.com/D168915
For what it's worth I have one small suggestion - it would be nice if there was some kind of feedback when you you press the "Apply setting and submit ping" button to let you know that you clicked it.
Description
•