fix(test): correct assertions in zipkin browser export tests#6566
Merged
pichlermarc merged 1 commit intoopen-telemetry:mainfrom Apr 7, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6566 +/- ##
==========================================
+ Coverage 95.74% 95.75% +0.01%
==========================================
Files 371 371
Lines 12514 12514
Branches 2962 2962
==========================================
+ Hits 11981 11983 +2
+ Misses 533 531 -2 🚀 New features to boost your workflow:
|
b170d5a to
3dc73c4
Compare
pichlermarc
approved these changes
Apr 7, 2026
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
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.
Which problem is this PR solving?
Several zipkin browser tests had assertions that silently passed without verifying anything. The tests appeared green but were not actually validating the behavior they claimed to test.
Short description of the changes
assert.ok(request.url, endpointUrl)was a no-op.assert.oktreats its second argument as a failure message, not a comparison value. The URL was never actually checked. Fixed toassert.strictEqual.Environment variable test could never work in browser. The test set
window.OTEL_EXPORTER_ZIPKIN_ENDPOINTexpecting the exporter to read it, butwindow.OTEL_*support was removed in the 2.x release (Feb 2025). The browsergetStringFromEnv()now unconditionally returnsundefined, so the exporter silently fell through to the default URL (localhost:9411). The brokenassert.okmasked this. Changed to useconfig.url, which is the only way to set a custom URL in the browser since 2.x.globalErrorHandlertests asserted against the wrong code path. HTTP 400 responses triggeronreadystatechange(notxhr.onerror), soglobalErrorHandlerwas never called. The old tests had nodonecallback, so the async assertions inside the export callback never ran. Rewritten to assertExportResultCode.FAILEDvia the export callback with properdonehandling.Added two new tests for previously uncovered paths:
sendBeaconreturningfalse(browser refused to queue) andxhr.onerrornetwork errors (DNS failure, CORS, connection refused) that callglobalErrorHandler.Type of change
How Has This Been Tested?
Checklist: