You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/07/12 03:27:10 UTC
[GitHub] [shardingsphere] runqi-zhao opened a new pull request, #19047: Add more unit test for DatabaseTypeFactory
runqi-zhao opened a new pull request, #19047:
URL: https://github.com/apache/shardingsphere/pull/19047
Fixes #16293.
Changes proposed in this pull request:
- add more unit test for DatabaseTypeFactory
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on a diff in pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
terrymanu commented on code in PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#discussion_r919621290
##########
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactory.java:
##########
@@ -52,4 +59,41 @@ public static DatabaseType getInstance(final String name) {
public static Collection<DatabaseType> getInstances() {
return ShardingSphereServiceLoader.getServiceInstances(DatabaseType.class);
}
+
Review Comment:
Why change main codes if the PR is add test cases only?
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] codecov-commenter commented on pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#issuecomment-1181292146
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19047](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2c3b476) into [master](https://codecov.io/gh/apache/shardingsphere/commit/f67b1f5e162fae73be5e5446902f5ee2c235e0c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f67b1f5) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #19047 +/- ##
============================================
- Coverage 59.44% 59.42% -0.02%
+ Complexity 2319 2318 -1
============================================
Files 3799 3797 -2
Lines 54646 54650 +4
Branches 9227 9231 +4
============================================
- Hits 32484 32478 -6
- Misses 19441 19451 +10
Partials 2721 2721
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...phere/infra/database/type/DatabaseTypeFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9kYXRhYmFzZS90eXBlL0RhdGFiYXNlVHlwZUZhY3RvcnkuamF2YQ==) | `23.52% <0.00%> (-76.48%)` | :arrow_down: |
| [...sphere/dbdiscovery/rule/DatabaseDiscoveryRule.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZGItZGlzY292ZXJ5L3NoYXJkaW5nc3BoZXJlLWRiLWRpc2NvdmVyeS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYmRpc2NvdmVyeS9ydWxlL0RhdGFiYXNlRGlzY292ZXJ5UnVsZS5qYXZh) | `66.25% <0.00%> (-2.89%)` | :arrow_down: |
| [...eadwritesplitting/rule/ReadwriteSplittingRule.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtcmVhZHdyaXRlLXNwbGl0dGluZy9zaGFyZGluZ3NwaGVyZS1yZWFkd3JpdGUtc3BsaXR0aW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3JlYWR3cml0ZXNwbGl0dGluZy9ydWxlL1JlYWR3cml0ZVNwbGl0dGluZ1J1bGUuamF2YQ==) | `56.45% <0.00%> (ø)` | |
| [...litting/rule/ReadwriteSplittingDataSourceRule.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtcmVhZHdyaXRlLXNwbGl0dGluZy9zaGFyZGluZ3NwaGVyZS1yZWFkd3JpdGUtc3BsaXR0aW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3JlYWR3cml0ZXNwbGl0dGluZy9ydWxlL1JlYWR3cml0ZVNwbGl0dGluZ0RhdGFTb3VyY2VSdWxlLmphdmE=) | `100.00% <0.00%> (ø)` | |
| [...ng/strategy/ReadwriteSplittingStrategyFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtcmVhZHdyaXRlLXNwbGl0dGluZy9zaGFyZGluZ3NwaGVyZS1yZWFkd3JpdGUtc3BsaXR0aW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3JlYWR3cml0ZXNwbGl0dGluZy9zdHJhdGVneS9SZWFkd3JpdGVTcGxpdHRpbmdTdHJhdGVneUZhY3RvcnkuamF2YQ==) | `80.00% <0.00%> (ø)` | |
| [...re/DatabaseDiscoveryDynamicDataSourceStrategy.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZGItZGlzY292ZXJ5L3NoYXJkaW5nc3BoZXJlLWRiLWRpc2NvdmVyeS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYmRpc2NvdmVyeS9hd2FyZS9EYXRhYmFzZURpc2NvdmVyeUR5bmFtaWNEYXRhU291cmNlU3RyYXRlZ3kuamF2YQ==) | | |
| [...rce/strategy/DynamicDataSourceStrategyFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9kYXRhc291cmNlL3N0cmF0ZWd5L0R5bmFtaWNEYXRhU291cmNlU3RyYXRlZ3lGYWN0b3J5LmphdmE=) | | |
| [...d/text/distsql/ral/common/hint/HintSourceType.java](https://codecov.io/gh/apache/shardingsphere/pull/19047/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi9oaW50L0hpbnRTb3VyY2VUeXBlLmphdmE=) | `42.85% <0.00%> (+42.85%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e9b1a...2c3b476](https://codecov.io/gh/apache/shardingsphere/pull/19047?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on a diff in pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
terrymanu commented on code in PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#discussion_r920720639
##########
shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactoryTest.java:
##########
@@ -57,4 +65,16 @@ public void assertGetInstances() {
assertThat(iterator.next(), instanceOf(SQLServerDatabaseType.class));
assertThat(iterator.next(), instanceOf(H2DatabaseType.class));
}
+
+ @Test
+ public void assertGetDatabaseTypeWithUrl() {
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:mysql://localhost:3306/test").getType(), is("MySQL"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:sqlite:test").getType(), is("SQL92"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:postgresql://localhost:5432/test").getType(), is("PostgreSQL"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:mariadb://localhost:3306/test").getType(), is("MariaDB"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:opengauss://localhost:5432/test").getType(), is("openGauss"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:oracle:thin:localhost:1522/test").getType(), is("Oracle"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:microsoft:sqlserver://localhost:1433; DatabaseName=test").getType(), is("SQLServer"));
+ assertThat(DatabaseTypeEngine.getDatabaseType("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1;DATABASE_TO_UPPER=false;MODE=MySQL").getType(), is("H2"));
Review Comment:
This method tests DatabaseTypeEngine, not DatabaseTypeFactory, and there are duplicated test cases in DatabaseTypeFactory. Can you remove it?
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on a diff in pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
terrymanu commented on code in PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#discussion_r920312082
##########
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactory.java:
##########
@@ -29,7 +29,7 @@
*/
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class DatabaseTypeFactory {
-
+
Review Comment:
Please do not change the original indent
##########
shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactoryTest.java:
##########
@@ -57,4 +57,17 @@ public void assertGetInstances() {
assertThat(iterator.next(), instanceOf(SQLServerDatabaseType.class));
assertThat(iterator.next(), instanceOf(H2DatabaseType.class));
}
+
+ @Test
+ public void assertGetDatabaseType() {
Review Comment:
Please rename the method to `assertGetInstance` if the target test method is `getInstance`
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] runqi-zhao commented on a diff in pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
runqi-zhao commented on code in PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#discussion_r919674370
##########
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactory.java:
##########
@@ -52,4 +59,41 @@ public static DatabaseType getInstance(final String name) {
public static Collection<DatabaseType> getInstances() {
return ShardingSphereServiceLoader.getServiceInstances(DatabaseType.class);
}
+
Review Comment:
In issues, add more unit test for DatabaseTypeFactory.
- DatabaseTypeFactory#getDatabaseType
But I didn't find this method, so I'll define it myself.
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] runqi-zhao commented on a diff in pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
runqi-zhao commented on code in PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047#discussion_r919948765
##########
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeFactory.java:
##########
@@ -52,4 +59,41 @@ public static DatabaseType getInstance(final String name) {
public static Collection<DatabaseType> getInstances() {
return ShardingSphereServiceLoader.getServiceInstances(DatabaseType.class);
}
+
Review Comment:
I understand. I am learning the code and have finished modifying it
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu merged pull request #19047: Add more unit test for DatabaseTypeFactory
Posted by GitBox <gi...@apache.org>.
terrymanu merged PR #19047:
URL: https://github.com/apache/shardingsphere/pull/19047
--
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@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org