1685402 - Support Custom Ping Unit Tests in FOG's JS and C++ languages

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Chris H-C :chutten]

Following the design in bug 1681742 (proposal doc), implement a mechanism to unit test custom pings in C++ and JS.

Don't forget tests (GTests and xpcshell) and documentation (in the Glean Book). Due to the (likely) language-specific nature of the API, I don't expect this to have to block on work in bug 1685401, but we should coordinate so that any changes to the design are reflected equivalently across the languages.

Comment 1

[Chris H-C :chutten]

I hear from Mossop that a way to do this would be the way nsITimer does with nsITimerCallback: apply the function annotation to the functor so that the JS API can take a function directly.

Then in C++ we could take std::function<void(const nsACString& aReason)> const& so we could take lambda expressions.

The idea'd be, for JS:

GleanPings.myPing.testBeforeNextSubmit((reason) => { /* ... */ });

C++:

mozilla::glean_pings::MyPing.TestBeforeNextSubmit([](const nsACString& aReason) { /* ... */ });

Comment 2

[Chris H-C :chutten]

Attachment: Bug 1685402 - FOG Rust testBeforeNextSubmit r?janerik!

Comment 3

[Chris H-C :chutten]

Attachment: Bug 1685402 - FOG JS and C++ TestBeforeNextSubmit r?janerik!

Two notes:

1. To make builds faster and hide the callback map, I've pulled the C++ impl
   into Ping.cpp.
   Doing this for the rest of the metrics is bug 1705394.
2. Doesn't wait on the queue to drain, actually writing metric values.
   This means that the callback may be running before values reach the db.
   This is surprisingly okay! Because the only way to check metrics values is
   testGetValue which, you guessed it, waits on the queue to drain.

Depends on D112219

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00dc350a9371
FOG Rust testBeforeNextSubmit r=janerik
https://hg.mozilla.org/integration/autoland/rev/f9eb32e83608
FOG JS and C++ TestBeforeNextSubmit r=janerik

Comment 5

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1685402) for build bustages in toolkit/components/glean/bindings/private/Ping.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=337022729&repo=autoland&lineNumber=31419

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=f9eb32e83608aeedb90cad1afded194cf97810ca

Backout:
https://hg.mozilla.org/integration/autoland/rev/6a25903b5467b601bb5a74dbb076ab1bf337b9bc

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/6a25903b5467

Comment 7

[Chris H-C :chutten]

Oh, I didn't ifdef out an unused function in the android build (GetCallbackMapLock). Easy enough to fix. I really do wish I could compile with warnings-as-errors locally, but there's way too much GTK nonsense for that ever to clear up.

Not sure quite what went wrong with the backout that caught bug 1704846 in the mix, but I'll see what I can do to land these both more sedately this time.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76ae5fc0cc74
FOG Rust testBeforeNextSubmit r=janerik
https://hg.mozilla.org/integration/autoland/rev/a0e7ceed0a81
FOG JS and C++ TestBeforeNextSubmit r=janerik

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/76ae5fc0cc74
https://hg.mozilla.org/mozilla-central/rev/a0e7ceed0a81