You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "Monilnarang (via GitHub)" <gi...@apache.org> on 2025/02/12 19:00:58 UTC
[PR] refactor: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Monilnarang opened a new pull request, #7160:
URL: https://github.com/apache/incubator-seata/pull/7160
- [ ] I have registered the PR [changes](../changes).
### Ⅰ. Describe what this PR did
Aim:
Improve the test code by avoiding code duplication, improving maintainability, and enhancing readability. By converting the test into a parameterized unit test, we reduce boilerplate code, make it easier to extend by simply adding new input values, and improve debugging by clearly identifying which test case fails instead of searching through individual assertions.
```
@Test
void getOrDefault() {
Assertions.assertEquals("Value", lowerCaseLinkHashMap.getOrDefault("Key", "abc"));
Assertions.assertEquals("Value", lowerCaseLinkHashMap.getOrDefault("key", "abc"));
Assertions.assertEquals("abc", lowerCaseLinkHashMap.getOrDefault("default", "abc"));
}
```
In the above in tests from LowerCaseLinkHashMapTest.java:
* The same method call (lowerCaseLinkHashMap.getOrDefault) is repeated multiple times with different inputs, making the test harder to maintain and extend.
* When a test fails, JUnit only shows which assertion failed, but not which specific input caused the failure.
* Adding new test cases requires copying and pasting another Assertions.assertEquals(...) statement instead of simply adding new data.
To accomplish this, I have retrofitted the test into a parameterized unit test. This reduces duplication, allows easy extension by simply adding new value sets, and makes debugging easier as it clearly indicates which test failed instead of requiring a search through individual assertions.
### Ⅱ. Does this pull request fix one issue?
### Ⅲ. Why don't you add test cases (unit test/integration test)?
### Ⅳ. Describe how to verify it
Successful tests run
### Ⅴ. Special notes for reviews
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] test: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] refactor: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#discussion_r1953648429
##########
common/src/test/java/org/apache/seata/common/util/LowerCaseLinkHashMapTest.java:
##########
@@ -122,11 +124,14 @@ void values() {
Assertions.assertArrayEquals(values.toArray(), lowerCaseLinkHashMap.values().toArray());
}
- @Test
- void getOrDefault() {
- Assertions.assertEquals("Value", lowerCaseLinkHashMap.getOrDefault("Key", "abc"));
- Assertions.assertEquals("Value", lowerCaseLinkHashMap.getOrDefault("key", "abc"));
- Assertions.assertEquals("abc", lowerCaseLinkHashMap.getOrDefault("default", "abc"));
+ @ParameterizedTest
+ @CsvSource(value = {
Review Comment:
Thank you, this is the first time I know this usage, thank you for your contribution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] test: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "Monilnarang (via GitHub)" <gi...@apache.org>.
Monilnarang commented on PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#issuecomment-2658421953
Added them as well. Please feel free to let me know if more changes are required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] refactor: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#issuecomment-2655226975
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7160?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 52.23%. Comparing base [(`2055817`)](https://app.codecov.io/gh/apache/incubator-seata/commit/205581771b46a1f5b72b47b008bc4f0663018141?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`049bf62`)](https://app.codecov.io/gh/apache/incubator-seata/commit/049bf62c0aa6e64076adef63c89435cafd22b408?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 2 commits behind head on 2.x.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 2.x #7160 +/- ##
============================================
+ Coverage 51.76% 52.23% +0.46%
- Complexity 6754 6810 +56
============================================
Files 1151 1154 +3
Lines 40975 41057 +82
Branches 4803 4811 +8
============================================
+ Hits 21212 21447 +235
+ Misses 17767 17572 -195
- Partials 1996 2038 +42
```
[see 41 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7160/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] test: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#issuecomment-2716135176
If you're using the DingTalk app, please send your DingTalk account to my email at `jianbin@apache.org`, and I will invite you to join the Seata developer group
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] test: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#discussion_r1955583483
##########
changes/zh-cn/2.x.md:
##########
Review Comment:
Please make sure your Github ID is in the list below
##########
changes/en-us/2.x.md:
##########
Review Comment:
Please make sure your Github ID is in the list below
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] test: Refactored tests in LowerCaseLinkHashMapTest to use parameterized unit testing [incubator-seata]
Posted by "Monilnarang (via GitHub)" <gi...@apache.org>.
Monilnarang commented on PR #7160:
URL: https://github.com/apache/incubator-seata/pull/7160#issuecomment-2658215212
Hi @funky-eyes,
Thanks for your comment. I have registered the PR in both the files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org