Open Bug 1756637 Opened 2 years ago Updated 10 months ago

Ensuring a ping was actually submitted is so important, `testBeforeNextSubmit` should help out with it

Categories

(Data Platform and Tools :: Glean: SDK, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: chutten, Unassigned)

Details

(Whiteboard: [telemetry:glean-rs:testing])

testBeforeNextSubmit is proving to be quite a decent way to test pings. However, it needs some help from the test writer to ensure things don't go awry. For example, this test will not fail:

add_task(async function test_should_fail() {
  GleanPings.myPing.testBeforeNextSubmit(reason => {
    Assert.ok(false);
  });
});

We've completely forgotten to actually submit the ping. This isn't great, so the new instrumentation testing docs being added in bug 1752357 recommend this pattern:

add_task(async function test_fails() {
  let submitted = false;
  GleanPings.myPing.testBeforeNextSubmit(reason => {
    submitted = true;
    Assert.ok(false);
  });
  Assert.ok(submitted);
});

This test will indeed fail if we forget to submit the ping. But it's an awkward manual process. Wouldn't it be nicer if it were more like:

add_task(async function test_times_out() {
  let submitted = GleanPings.myPing.testBeforeNextSubmit(reason => {
    Assert.ok(false);
  });
  await submitted; // Will wait forever if ping not submitted
});

Or maybe even some other better design we haven't yet come up with?

This ping is about designing and implementing a better ping testing design that makes it more ergonomic across the various SDK languages for instrumentation tests to assert that the ping was actually submitted and the testBeforeNextSubmit closure was called.

Note that this is the exact behaviour that Glean.js has (see here). The FOG JS API was quite different because submit was synch (if I remember right): not sure why we still need validatorRun there though

Priority: -- → P3
Whiteboard: [telemetry:glean-rs:testing]

Another potentially-related point of interest: now that testBeforeNextSubmit can be called within testBeforeNextSubmit at least in FOG JS, we can use self-referential callbacks to capture multiple ping payloads in a row. At least one person thinks this is a pattern with value.

Perhaps this is something that'd dovetail with this work? Instead of "just" a submitted promise, it could be a submitted promise that resolves with the (full?!) ping payloads that were submitted?

You need to log in before you can comment on or make changes to this bug.