-
Notifications
You must be signed in to change notification settings - Fork 8.9k
test: fix JUnit test method access modifiers and annotations #7635
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
base: 2.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there similar cases elsewhere?
If so, it would be good to address all of them in this PR.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.x #7635 +/- ##
============================================
- Coverage 61.41% 61.41% -0.01%
- Complexity 682 684 +2
============================================
Files 1324 1324
Lines 50047 50047
Branches 5910 5910
============================================
- Hits 30738 30735 -3
- Misses 16531 16542 +11
+ Partials 2778 2770 -8 🚀 New features to boost your workflow:
|
Thanks for your suggestion! I’ve checked and fixed all similar cases in the test classes. Could you please take another look when you get a chance? |
Please use JUnit5 instead of JUnit4. |
Done. Replaced all JUnit4 references with JUnit5. Please take another look when you get a chance. |
@YongGoose I've updated the PR based on your feedback. Could you please take another look when you have time? Thanks! |
The CI failure in the seata-config-zk module only occurs in the Java 8 build and is unrelated to my changes. The same tests pass successfully in Java 17 and Java 21, indicating a compatibility issue between ZooKeeper client and Java 8 in the CI environment—not a code problem. |
...cd3/src/test/java/org/apache/seata/discovery/registry/etcd3/EtcdRegistryServiceImplTest.java
Show resolved
Hide resolved
Hi @YongGoose , |
@YongGoose Pls take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please apply spotless :)
Once you’ve fixed the Spotless issues, please ping me and I’ll give my approval.
EtcdListener etcdListener = new EtcdListener(); | ||
registryService.subscribe(CLUSTER_NAME, etcdListener); | ||
// 3.delete instance,see if the listener can be notified | ||
registryService.subscribe(DEFAULT_TX_GROUP, etcdListener); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, was there any particular reason for using DEFAULT_TX_GROUP instead of CLUSTER_NAME?
Ⅰ. Describe what this PR did
This PR corrects JUnit test method access modifiers and annotation usage:
@BeforeEach
/@AfterEach
method visibility in JUnit 5 tests (private → public)@Rule
field access modifier in JUnit 4 tests (private → public)@Test
annotation from utility methodsThese changes ensure proper JUnit framework functionality and test execution.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews