Skip to content

Conversation

@acured
Copy link
Collaborator

@acured acured commented Nov 20, 2025

add test code to expand the test code coverage to cover all paths inside the agentops' _trace_context_sync function.

…ide the agentops' _trace_context_sync function.
@acured acured marked this pull request as ready for review November 20, 2025 05:42
@acured acured requested a review from ultmaster November 20, 2025 05:42
)

def otlp_traces_endpoint(self) -> str:
return "dump://"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this won't cause errors? How do you check whether the traces have been dumped?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it, added a mocked service.

from agentlightning.utils import otlp


class MockLightningStoreService:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest MockOTLPService

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

tracer.teardown()


def test_agentops_trace_with_store_or_not():
Copy link
Contributor

Choose a reason for hiding this comment

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

use pytest.parametrize to split into three tests maybe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@ultmaster
Copy link
Contributor

/ci

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

🚀 CI Watcher for correlation id-3585196667-mihb45ca triggered by comment 3585196667
🏃‍♀️ Tracking 1 workflow run(s):

✅ All runs completed.

@ultmaster ultmaster merged commit 3a3bfee into microsoft:main Nov 27, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants