Skip to content

Add testable examples to pdata module #13244

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

minimAluminiumalism
Copy link

Description

Add testable examples to pdata module. I've only started with the traces/metrics/logs examples.

Link to tracking issue

Feat #13145

Testing

Documentation

Copy link
Member

@JaredTan95 JaredTan95 left a comment

Choose a reason for hiding this comment

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

nit: How about move those examples into doc_test.go separately, following https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/doc_test.go

Copy link

codecov bot commented Jun 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.45%. Comparing base (4ddf2cc) to head (ea13a4d).
Report is 25 commits behind head on main.

❌ Your project check has failed because the head coverage (86.45%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13244      +/-   ##
==========================================
- Coverage   91.28%   86.45%   -4.83%     
==========================================
  Files         511      524      +13     
  Lines       28802    31992    +3190     
==========================================
+ Hits        26291    27660    +1369     
- Misses       1994     3553    +1559     
- Partials      517      779     +262     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@minimAluminiumalism
Copy link
Author

@bogdandrutu It seems that the code coverage is too low so I've added more examples to improve that. Please review it.

@minimAluminiumalism
Copy link
Author

minimAluminiumalism commented Jun 25, 2025

@bogdandrutu Other workflow failures have been fixed. But the codecov workflow still failed although I've add more examples to improve the coverage. Actually I've test the coverage of pdata in my local dev environment and the example code has high coverage over 90%. Can you check that?

@dmathieu
Copy link
Member

I think you've been slighlyt mixing two things here.
Example tests are meant to provide an example usage of a package, not to provide full testing/improve code coverage.

The code coverage low rate is something we can fix, but with new tests, not with new examples.
I'd recommend writing the new tests in a new PR, not this one.

@minimAluminiumalism
Copy link
Author

I think you've been slighlyt mixing two things here. Example tests are meant to provide an example usage of a package, not to provide full testing/improve code coverage.

The code coverage low rate is something we can fix, but with new tests, not with new examples. I'd recommend writing the new tests in a new PR, not this one.

I fully agree with you. Could you review this PR and re-run the CI pipeline, omitting the code coverage checks?

@dmathieu
Copy link
Member

The code coverage checks are non-blocking. They can't be skipped, but can fail and not prevent a PR from being merged.

@dmathieu dmathieu added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 25, 2025
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.

5 participants