test: add unit tests for trace processing helpers - #2514
Open
ZayanKhan-12 wants to merge 1 commit into
Open
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Adds direct unit tests for the pure helpers exported from src/trace-processing/parse.ts: - traceResultIsSuccess: accepts real parsed trace results (with and without insights) and rejects TraceParseError objects, including a near-miss where the error message contains the discriminant key name. - parseRawTraceBuffer: covers the previously untested error branches for empty buffers, invalid JSON, and valid JSON that is not a trace. - getInsightOutput: covers the formatted output for a known insight and all three error branches (no insights, unknown insight set id, unknown insight name).
ZayanKhan-12
force-pushed
the
chore/tests-trace-guard
branch
from
August 8, 2026 21:46
7e939f4 to
14ccacf
Compare
Comment on lines
+128
to
+130
| if ('error' in insight) { | ||
| assert.fail(`Unexpected insight error: ${insight.error}`); | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| if ('error' in insight) { | |
| assert.fail(`Unexpected insight error: ${insight.error}`); | |
| } | |
| assert(!('error' in insight`),Unexpected insight error: ${insight.error}`); |
Comment on lines
+118
to
+121
| it('checks for the parsedTrace key, not the error message contents', () => { | ||
| const error: TraceParseError = {error: 'parsedTrace'}; | ||
| assert.strictEqual(traceResultIsSuccess(error), false); | ||
| }); |
Collaborator
There was a problem hiding this comment.
Suggested change
| it('checks for the parsedTrace key, not the error message contents', () => { | |
| const error: TraceParseError = {error: 'parsedTrace'}; | |
| assert.strictEqual(traceResultIsSuccess(error), false); | |
| }); |
Comment on lines
+85
to
+88
| if (traceResultIsSuccess(result)) { | ||
| assert.fail('Expected a parse error for a non-trace JSON input.'); | ||
| } | ||
| assert.ok(result.error.length > 0); |
Collaborator
There was a problem hiding this comment.
Suggested change
| if (traceResultIsSuccess(result)) { | |
| assert.fail('Expected a parse error for a non-trace JSON input.'); | |
| } | |
| assert.ok(result.error.length > 0); | |
| assert('error' in result); | |
| assert.ok(result.error.length > 0); |
Comment on lines
+75
to
+78
| if (traceResultIsSuccess(result)) { | ||
| assert.fail('Expected a parse error for invalid JSON input.'); | ||
| } | ||
| assert.match(result.error, /JSON/); |
Collaborator
There was a problem hiding this comment.
Suggested change
| if (traceResultIsSuccess(result)) { | |
| assert.fail('Expected a parse error for invalid JSON input.'); | |
| } | |
| assert.match(result.error, /JSON/); | |
| assert('error' in result); | |
| assert.match(result.error, /JSON/); |
Comment on lines
+27
to
+29
| if (!traceResultIsSuccess(result)) { | ||
| assert.fail(`Unexpected trace parse error: ${result.error}`); | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| if (!traceResultIsSuccess(result)) { | |
| assert.fail(`Unexpected trace parse error: ${result.error}`); | |
| } | |
| assert(traceResultIsSuccess(result), `Unexpected trace parse error: ${result.error}`); |
| }); | ||
| }); | ||
|
|
||
| describe('traceResultIsSuccess', () => { |
Collaborator
There was a problem hiding this comment.
For this function we should just use a object that mimics the trace, so we can control it.
Also we can't use parseTrace as that itself include it.
Comment on lines
47
to
49
| if ('error' in result) { | ||
| assert.fail(`Unexpected parse failure: ${result.error}`); | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| assert(!('error' in result), `Unexpected parse failure: ${result.error}`); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
src/trace-processing/parse.tsexports several pure, browser-free helpers, buttests/trace-processing/parse.test.tsonly exercised the happy parse path, the trace summary snapshot, and theundefined-buffer error. This PR adds direct unit tests for the remaining exported helpers and branches, so regressions in the discriminant logic or the error contracts are caught without needing a browser.Coverage added
traceResultIsSuccesstruefor the result of a real successful parse (fixture trace)truefor aTraceResultwhoseinsightsisnull(the guard must only depend onparsedTrace)falsefor the result of a failed parsefalsefor aTraceParseErrorobjectfalsefor a near-miss where the error message text is'parsedTrace'— the guard checks the key, not valuesparseRawTraceBuffer(previously untested error branches)'Decoding the trace buffer returned an empty string.'TraceParseErrorinstead of throwingTraceParseErrorinstead of throwinggetInsightOutput(previously untested)NAVIGATION_0/LCPBreakdownfrom theweb-dev-with-commitfixture)Tests only — no changes under
src/, and no new snapshots (the insight output is asserted with a targeted match so devtools-frontend formatter updates don't churn a snapshot).Testing
npm run build— cleannode scripts/test.js tests/trace-processing/parse.test.ts— 15/15 pass (3 suites)npm run test:no-build— full suite green (exit 0)npm run check-format— eslint + prettier clean🤖 Generated with Claude Code