Skip to content

Update tracecontext integration test gitref#4448

Merged
xrmx merged 1 commit intoopen-telemetry:mainfrom
jomcgi:fix-issue-4105
Feb 26, 2025
Merged

Update tracecontext integration test gitref#4448
xrmx merged 1 commit intoopen-telemetry:mainfrom
jomcgi:fix-issue-4105

Conversation

@jomcgi
Copy link
Contributor

@jomcgi jomcgi commented Feb 26, 2025 •
edited by emdneto
Loading

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 26, 2025 •
edited
Loading

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 •
edited
Loading

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

How tests passed locally?

emdneto
emdneto reviewed Feb 26, 2025
View reviewed changes
@jomcgi
fix: update tracecontext integration test gitref
91c3114 
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
@xrmx xrmx moved this to Done in Python PR digest Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx approved these changes

@emdneto emdneto emdneto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Python PR digest
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update tracecontext tests

3 participants

@jomcgi @emdneto @xrmx