e2e test for background error telemetry #8717
Conversation
fungairino
force-pushed
the
e2e-background
branch
from
89fbd0e to
10cbeb1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8717 +/- ##
==========================================
+ Coverage 73.69% 74.07% +0.37%
==========================================
Files 1344 1342 -2
Lines 41566 41387 -179
Branches 7775 7753 -22
==========================================
+ Hits 30633 30658 +25
+ Misses 10933 10729 -204 ☔ View full report in Codecov by Sentry. |
Playwright test results
Details
Skipped testschrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options |
| @@ -26,54 +26,68 @@ async function waitForBackgroundPageRequest( | |||
| return offscreenPage?.waitForRequest(errorServiceEndpoint); | |||
| } | |||
|
|
|||
| async function getSentErrors( | |||
There was a problem hiding this comment.
| async function getSentErrors( | |
| async function getForwardedErrors( |
or
| async function getSentErrors( | |
| async function getErrorsFromRequest( |
I don't know about anyone else, but my brain lumps "getSent" together instead of "sentErrors" together
| @@ -82,6 +96,100 @@ test("can report application error to telemetry service", async ({ | |||
| message: expect.any(String), | |||
| kind: expect.any(String), | |||
| }), | |||
| // Stack and message are duplicated | |||
There was a problem hiding this comment.
Not sure what the intended goal with this one is. Is this a bug? Does message == stack? If so, are we missing the stack or the message?
I could at least see a comment like this being better suited for where we add the error message in the first place as opposed to a test, e.g.
pixiebrix-extension/src/telemetry/logging.ts
Line 430 in 3b6d29c
But at best I'd like to clarify if this is a problem or not, and create a ticket if so.
| context, | ||
| extensionId, | ||
| }) => { | ||
| const errorServiceEndpoint = "https://browser-intake-datadoghq.com/api/v2/*"; |
There was a problem hiding this comment.
nit: opportunity to extract errorServiceEndpoint
|
|
||
| await test.step("Mock the registry endpoint to return a bad response, and mock errorService calls", async () => { | ||
| await context.route( | ||
| "https://app.pixiebrix.com/api/registry/bricks/", |
There was a problem hiding this comment.
I would extract this endpoint into a constant called something like endpointCalledFromServiceWorker to clarify what makes this test different from the first, and how this test covers the service worker case.
| ) { | ||
| // TODO: due to Datadog SDK implementation, it will take ~30 seconds for the | ||
| // request to be sent. We should figure out a way to induce the request to be sent sooner. | ||
| // See this datadog support request: https://help.datadoghq.com/hc/en-us/requests/1754158 |
There was a problem hiding this comment.
❤️ thanks again to reaching out to support
| ); | ||
| }); | ||
|
|
||
| await page.goto(getBaseExtensionConsoleUrl(extensionId)); |
There was a problem hiding this comment.
It might be too early to extract at 2x usages, but maybe if we end up adding a third test with this shape, we should extract this logic into something reusable.
* e2e test for background error telemetry * pr feedback * rename * fix error not being emitted in bg
What does this PR do?
Future Work
Checklist
For more information on our expectations for the PR process, see the
code review principles doc