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

Update tracecontext integration test gitref #4448

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

jomcgi
Copy link
Contributor

@jomcgi jomcgi commented Feb 26, 2025

Description

We're currently using an outdated version of the W3C test, this results in warnings in CI.
Updated gitref in scripts/tracecontext-integration-test.sh to latest from wc3/trace-context.
Skipped changed test that is incompatible with our implementation (we drop duplicated keys which is optional in the spec).
Added fixme comment to track setting flags for the test suite when they are available (wc3 issue)

Fixes #4105
Closes #4109

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Executed test suite locally

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link

linux-foundation-easycla bot commented Feb 26, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: jomcgi / name: Joe McGinley (91c3114)

@jomcgi jomcgi changed the title fix: update tracecontext integration test gitref [WIP] fix: update tracecontext integration test gitref Feb 26, 2025
@jomcgi jomcgi changed the title [WIP] fix: update tracecontext integration test gitref Update tracecontext integration test gitref Feb 26, 2025
@jomcgi jomcgi marked this pull request as ready for review February 26, 2025 09:20
@jomcgi jomcgi requested a review from a team as a code owner February 26, 2025 09:20
@jomcgi
Copy link
Contributor Author

jomcgi commented Feb 26, 2025

Apologies this is my first PR - tests were passing locally but are failing in CI with tox.
There are some options for upgrading and resolving the error:

  1. Reduce strictness level to 1 (skips this test but we also lose other coverage that matches our implementation)
  2. Filter tests - bash or python processing to extract tests we would like to run
  3. Wait for W3C issue to be resolved (unknown timeline) so that flags are properly supported

Option 2 works with:

TEST_TO_IGNORE="test_tracestate_duplicated_keys"
FILTERED_TESTS=$(python -c "
import unittest

from test import TraceContextTest

tests = unittest.TestLoader().loadTestsFromTestCase(TraceContextTest)
test_names = [
    f'TraceContextTest.{t._testMethodName}'
    for t in tests
    if t._testMethodName != '${TEST_TO_IGNORE}'
]
print(' '.join(test_names))
"  | grep "TraceContextTest")
python test.py http://127.0.0.1:5000/verify-tracecontext AdvancedTest TraceContext2Test $FILTERED_TESTS

Guidance would be appreciated - I can follow up with whatever is recommended.

@emdneto
Copy link
Member

emdneto commented Feb 26, 2025

Apologies this is my first PR - tests were passing locally but are failing in CI with tox.

How tests passed locally?

Update gitref to latest from https://github.com/w3c/trace-context
Skip changed test that is incompatible with our implementation
Add fixme comment to track setting flags for the test suite when
they are available
@xrmx xrmx merged commit ac7329c into open-telemetry:main Feb 26, 2025
384 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update tracecontext tests
3 participants