Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky global ErrorHandler delegation test #1829

Merged
merged 4 commits into from
Apr 21, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 20, 2021

Remove multi-goroutine approach and just test what needs to be tested. Meaning this:

--- FAIL: TestHandlerTestSuite (0.05s)
    --- FAIL: TestHandlerTestSuite/TestNoDropsOnDelegate (0.01s)
        handler_test.go:121: 
            	Error Trace:	handler_test.go:121
            	Error:      	Received unexpected error:
            	            	no errors sent in 10ms
            	Test:       	TestHandlerTestSuite/TestNoDropsOnDelegate
            	Messages:   	starting error stream
FAIL
coverage: 1.5% of statements in ./...
FAIL	go.opentelemetry.io/otel/api/global	0.172s

should be fixed and not cause false positives.

Resolves #907

Remove multi-goroutine approach and just test what needs to be tested.
@MrAlias MrAlias added bug Something isn't working pkg:testing Related to testing or a testing package labels Apr 20, 2021
@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 20, 2021
@MrAlias MrAlias changed the title Resolve flaky global ErrorHandler delegation test Fix flaky global ErrorHandler delegation test Apr 20, 2021
@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #1829 (e71537f) into main (e43d9c0) will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #1829   +/-   ##
=====================================
  Coverage   78.3%   78.4%           
=====================================
  Files        135     135           
  Lines       7246    7246           
=====================================
+ Hits        5680    5682    +2     
+ Misses      1318    1317    -1     
+ Partials     248     247    -1     
Impacted Files Coverage Δ
exporters/trace/jaeger/jaeger.go 88.5% <0.0%> (-1.0%) ⬇️
sdk/trace/batch_span_processor.go 83.9% <0.0%> (+1.5%) ⬆️
exporters/otlp/otlpgrpc/connection.go 92.5% <0.0%> (+1.8%) ⬆️

@Aneurysm9
Copy link
Member

Merging this with a single approval and prior to 24h as it is straightforward, confined to tests, and will eliminate a pain in our neck for other PRs in flight.

@Aneurysm9 Aneurysm9 merged commit 738ef11 into open-telemetry:main Apr 21, 2021
@MrAlias MrAlias deleted the 907 branch April 21, 2021 19:21
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:testing Related to testing or a testing package Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix api/global test flake
3 participants