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 order dependent tests in SofaRpcFallbackRegistryTest #3282

Conversation

SaaiVenkat
Copy link
Contributor

Describe what this PR does / why we need it

  • In sentinel-adapter/sentinel-sofa-rpc-adapter, the unit test SofaRpcFallbackRegistryTest.testDefaultFallback() will fail when run after the unit test SofaRpcFallbackRegistryTest.testCustomFallback() because it pollutes state shared among tests.
  • It may be good to clean this state pollution so that this tests and some other tests do not fail in the future due to the shared state polluted by this test.

Does this pull request fix one issue?

Fixes #3281

Describe how you did it

  • As mentioned in the issue, I have cleaned the polluted states SofaRpcFallback.providerFallback and SofaRpcFallback.consumerFallback to its default value after the test SofaRpcFallbackRegistryTest.testCustomFallback() is executed.

Describe how to verify it

  • With the proposed fix, the test does not pollute the shared state.
  • All the tests pass when run in the any order.

Copy link
Collaborator

@LearningGp LearningGp left a comment

Choose a reason for hiding this comment

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

LGTM

@LearningGp
Copy link
Collaborator

Thanks for contributing!

@LearningGp LearningGp merged commit 6879fdd into alibaba:master Dec 24, 2023
7 checks passed
z-soulx pushed a commit to z-soulx/Sentinel that referenced this pull request May 27, 2024
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.

[BUG] SofaRpcFallbackRegistryTest has order dependent tests
2 participants