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

Use subtests in the trace package #2594

Merged
merged 2 commits into from Feb 9, 2022
Merged

Use subtests in the trace package #2594

merged 2 commits into from Feb 9, 2022

Conversation

dmathieu
Copy link
Member

@dmathieu dmathieu commented Feb 9, 2022

As I was looking at tests within the trace package, I noticed they use range test cases, but without sub tests, making it harder to run a single test.

@dmathieu
Copy link
Member Author

dmathieu commented Feb 9, 2022

As this only changes tests, it shouldn't need a changelog entry.

@@ -26,74 +26,91 @@ import (
// Taken from the W3C tests:
// https://github.com/w3c/trace-context/blob/dcd3ad9b7d6ac36f70ff3739874b73c11b0302a1/test/test_data.json
var testcases = []struct {
name string
Copy link
Member Author

@dmathieu dmathieu Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, we could use in as the name of the subtest here. I've added an explicit name though, as I believe it makes it clearer what the case's goal is.

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 9, 2022
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #2594 (99474fa) into main (9f42a81) will decrease coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2594     +/-   ##
=======================================
- Coverage   76.2%   76.1%   -0.1%     
=======================================
  Files        173     173             
  Lines      12238   12238             
=======================================
- Hits        9328    9322      -6     
- Misses      2667    2673      +6     
  Partials     243     243             
Impacted Files Coverage Δ
sdk/trace/batch_span_processor.go 80.2% <0.0%> (-1.9%) ⬇️
exporters/jaeger/jaeger.go 92.6% <0.0%> (-0.9%) ⬇️

@Aneurysm9
Copy link
Member

This is a test-only change with several approvals already; merging prior to 24h.

@Aneurysm9 Aneurysm9 merged commit 010ca4f into open-telemetry:main Feb 9, 2022
@dmathieu dmathieu deleted the trace-subtests branch February 9, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants