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

fix TracingSegmentRunner #688

Merged
merged 3 commits into from
May 12, 2024
Merged

fix TracingSegmentRunner #688

merged 3 commits into from
May 12, 2024

Conversation

JoeCqupt
Copy link
Contributor

@JoeCqupt JoeCqupt commented May 11, 2024

Fix

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.
    TracingSegmentRunner override method withAfters made Junit @after annotation not work

@wu-sheng wu-sheng added this to the 9.3.0 milestone May 11, 2024
@wu-sheng wu-sheng added the bug Something isn't working label May 11, 2024
@lujiajing1126
Copy link
Contributor

Nice catch. But you have to verify all test cases work as expected. I notice we've declared many cleanups or teardowns. But they are actually not running at all.

@CodePrometheus
Copy link

I think org.apache.skywalking.apm.agent.core.test.tools.TracingSegmentRunner#withAfters also needs to be modified.

Copy link

@CodePrometheus CodePrometheus left a comment

Choose a reason for hiding this comment

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

Now I think all cleanup and teardown could work correctly, the code LGTM, thanks @JoeCqupt for your attentive discovery and don't forget to update CHANGES, let's wait for other teachers to review.

@wu-sheng
Copy link
Member

Now I think all cleanup and teardown could work correctly, the code LGTM, thanks @JoeCqupt for your attentive discovery and don't forget to update CHANGES, let's wait for other teachers to review.

As no feature code change, no changelog required.

@wu-sheng wu-sheng merged commit 0bf535e into apache:main May 12, 2024
190 checks passed
@JoeCqupt JoeCqupt deleted the patch-1 branch May 13, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants