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 2020/02/27 08:58:32 UTC

[GitHub] [incubator-shardingsphere] haetao opened a new pull request #4498: Add Unit test case to NacosConfigInstanceTest

haetao opened a new pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498
 
 
   Fixes #4450 .
   
   Changes proposed in this pull request:
   -
   -
   -
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385564779
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/nonExistentKey")));
 
 Review comment:
   Please use assertNull

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591948708
 
 
   ## Pull Request Test Coverage Report for [Build 9853](https://coveralls.io/builds/29022409)
   
   * **0** of **0**   changed or added relevant lines in **0** files are covered.
   * **654** unchanged lines in **15** files lost coverage.
   * Overall coverage increased (+**0.2%**) to **59.048%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/ddl/column/alter/AddColumnDefinitionSegment.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fddl%2Fcolumn%2Falter%2FAddColumnDefinitionSegment.java#L52) | 1 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/ddl/constraint/ConstraintDefinitionSegment.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fddl%2Fconstraint%2FConstraintDefinitionSegment.java#L54) | 1 | 0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29022409/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/TableTokenGenerator.java](https://coveralls.io/builds/29022409/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FTableTokenGenerator.java#L102) | 6 | 85.0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/ddl/AlterTableStatement.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fddl%2FAlterTableStatement.java#L37) | 7 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/SQL92DDLVisitor.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FSQL92DDLVisitor.java#L55) | 32 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/SQLServerDDLVisitor.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-sqlserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FSQLServerDDLVisitor.java#L69) | 40 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/OracleDDLVisitor.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-oracle%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FOracleDDLVisitor.java#L70) | 53 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDDLVisitor.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDDLVisitor.java#L71) | 53 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDDLVisitor.java](https://coveralls.io/builds/29022409/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDDLVisitor.java#L80) | 64 | 0% |
   <!-- | **Total:** | **654** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29022409/badge)](https://coveralls.io/builds/29022409) |
   | :-- | --: |
   | Change from base [Build 942](https://coveralls.io/builds/28995512): |  0.2% |
   | Covered Lines: | 10615 |
   | Relevant Lines: | 17977 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385566801
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/nonExistentKey")));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/test")));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
 
 Review comment:
   Please do not use Class.InnerClass, just put Class.InnerClass into import statements

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu merged pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591948708
 
 
   ## Pull Request Test Coverage Report for [Build 9840](https://coveralls.io/builds/29017102)
   
   * **0** of **0**   changed or added relevant lines in **0** files are covered.
   * **630** unchanged lines in **16** files lost coverage.
   * Overall coverage increased (+**0.2%**) to **59.075%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/ddl/column/alter/AddColumnDefinitionSegment.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fddl%2Fcolumn%2Falter%2FAddColumnDefinitionSegment.java#L52) | 1 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/ddl/constraint/ConstraintDefinitionSegment.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fddl%2Fconstraint%2FConstraintDefinitionSegment.java#L54) | 1 | 0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29017102/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dml/item/ShorthandProjectionSegment.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdml%2Fitem%2FShorthandProjectionSegment.java#L49) | 3 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/ddl/AlterTableStatement.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fddl%2FAlterTableStatement.java#L37) | 7 | 0% |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/TableTokenGenerator.java](https://coveralls.io/builds/29017102/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FTableTokenGenerator.java#L103) | 9 | 87.95% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/SQL92DDLVisitor.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FSQL92DDLVisitor.java#L55) | 32 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/SQLServerDDLVisitor.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-sqlserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FSQLServerDDLVisitor.java#L69) | 40 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/OracleDDLVisitor.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-oracle%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FOracleDDLVisitor.java#L70) | 53 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDDLVisitor.java](https://coveralls.io/builds/29017102/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDDLVisitor.java#L71) | 53 | 0% |
   <!-- | **Total:** | **630** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29017102/badge)](https://coveralls.io/builds/29017102) |
   | :-- | --: |
   | Change from base [Build 942](https://coveralls.io/builds/28995512): |  0.2% |
   | Covered Lines: | 10620 |
   | Relevant Lines: | 17977 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385106728
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -101,6 +103,68 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
         nacosConfigCenterRepository.watch("/sharding/test", listener);
         Assert.assertEquals(expectValue, actualValue[0]);
     }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/nonExistentKey"));
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/test"));
 
 Review comment:
   we can `import static org.junit.Assert.assertThat;` and use `assertEquals` directly, keep the same with other test class.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385567316
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/nonExistentKey")));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/test")));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(expectValue)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+    
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualValue[0] = dataChangedEvent.getValue();
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        assertThat(expectValue, is(actualValue[0]));
+        assertThat(DataChangedEvent.ChangedType.UPDATED, is(actualType[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertWatchDeletedChangedType() {
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(null)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+    
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        assertThat(DataChangedEvent.ChangedType.UPDATED, is(actualType[0]));
 
 Review comment:
   The first arg of assertThat is `actual`, second arg is `expected`.
   It is reversed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] haetao commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
haetao commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385494556
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +101,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertEquals(expectValue, actualValue[0]);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertEquals(null, nacosConfigCenterRepository.get("/sharding/nonExistentKey"));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        assertEquals(null, nacosConfigCenterRepository.get("/sharding/test"));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(expectValue)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+    
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualValue[0] = dataChangedEvent.getValue();
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        assertEquals(expectValue, actualValue[0]);
+        assertEquals(DataChangedEvent.ChangedType.UPDATED, actualType[0]);
 
 Review comment:
   Ok, I'll mdify 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591948708
 
 
   ## Pull Request Test Coverage Report for [Build 9831](https://coveralls.io/builds/29000013)
   
   * **0** of **0**   changed or added relevant lines in **0** files are covered.
   * **3** unchanged lines in **1** file lost coverage.
   * Overall coverage increased (+**0.01%**) to **58.893%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29000013/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   <!-- | **Total:** | **3** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29000013/badge)](https://coveralls.io/builds/29000013) |
   | :-- | --: |
   | Change from base [Build 942](https://coveralls.io/builds/28995512): |  0.01% |
   | Covered Lines: | 10616 |
   | Relevant Lines: | 18026 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591948708
 
 
   ## Pull Request Test Coverage Report for [Build 9835](https://coveralls.io/builds/29016218)
   
   * **0** of **0**   changed or added relevant lines in **0** files are covered.
   * **3** unchanged lines in **1** file lost coverage.
   * Overall coverage increased (+**0.01%**) to **58.893%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29016218/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   <!-- | **Total:** | **3** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29016218/badge)](https://coveralls.io/builds/29016218) |
   | :-- | --: |
   | Change from base [Build 942](https://coveralls.io/builds/28995512): |  0.01% |
   | Covered Lines: | 10616 |
   | Relevant Lines: | 18026 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385116948
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -101,6 +103,68 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
         nacosConfigCenterRepository.watch("/sharding/test", listener);
         Assert.assertEquals(expectValue, actualValue[0]);
     }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/nonExistentKey"));
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/test"));
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(expectValue)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualValue[0] = dataChangedEvent.getValue();
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        Assert.assertEquals(expectValue, actualValue[0]);
+        Assert.assertEquals(DataChangedEvent.ChangedType.UPDATED, actualType[0]);
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertWatchDeletedChangedType() {
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(null)))
 
 Review comment:
   AdditionalAnswers also import static class.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls commented on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591948708
 
 
   ## Pull Request Test Coverage Report for [Build 9827](https://coveralls.io/builds/28996830)
   
   * **0** of **0**   changed or added relevant lines in **0** files are covered.
   * **3** unchanged lines in **1** file lost coverage.
   * Overall coverage increased (+**0.01%**) to **58.893%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/28996830/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   <!-- | **Total:** | **3** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/28996830/badge)](https://coveralls.io/builds/28996830) |
   | :-- | --: |
   | Change from base [Build 942](https://coveralls.io/builds/28995512): |  0.01% |
   | Covered Lines: | 10616 |
   | Relevant Lines: | 18026 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385566162
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
 
 Review comment:
   The first arg of assertThat is `actual`, second arg is `expected`.
   It is reversed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385102813
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -36,10 +37,11 @@
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.doThrow;
 
 public class NacosConfigInstanceTest {
 
 Review comment:
   Class should be final.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] menghaoranss commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
menghaoranss commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385483155
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +101,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertEquals(expectValue, actualValue[0]);
+    }
+
 
 Review comment:
   Blank lines need to be indented，please check.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385566273
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/nonExistentKey")));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        assertThat(null, is(nacosConfigCenterRepository.get("/sharding/test")));
 
 Review comment:
   Please use assertNull

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] haetao commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
haetao commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385135842
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -101,6 +103,68 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
         nacosConfigCenterRepository.watch("/sharding/test", listener);
         Assert.assertEquals(expectValue, actualValue[0]);
     }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/nonExistentKey"));
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        Assert.assertEquals(null, nacosConfigCenterRepository.get("/sharding/test"));
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(expectValue)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualValue[0] = dataChangedEvent.getValue();
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        Assert.assertEquals(expectValue, actualValue[0]);
+        Assert.assertEquals(DataChangedEvent.ChangedType.UPDATED, actualType[0]);
+    }
+
+    @Test
+    @SneakyThrows
+    public void assertWatchDeletedChangedType() {
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(null)))
 
 Review comment:
   ![image](https://user-images.githubusercontent.com/46469719/75450959-aec9e200-59aa-11ea-982d-20e2dc5e414a.png)
   'AdditionalAnswers' can not be used  'static import' to import.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] codecov-io commented on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591945952
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=h1) Report
   > Merging [#4498](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/a569f0e561bf1fc0fe52db31734e23e9c5ea4fdd?src=pr&el=desc) will **increase** coverage by `0.01%`.
   > The diff coverage is `14.8%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4498      +/-   ##
   ============================================
   + Coverage     55.23%   55.24%   +0.01%     
     Complexity      330      330              
   ============================================
     Files           953      953              
     Lines         18027    18026       -1     
     Branches       3408     3408              
   ============================================
   + Hits           9957     9959       +2     
   + Misses         7425     7422       -3     
     Partials        645      645
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...phere/sql/parser/visitor/impl/MySQLDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW15c3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9NeVNRTERDTFZpc2l0b3IuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...e/sql/parser/visitor/impl/SQLServerDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXNxbHNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci92aXNpdG9yL2ltcGwvU1FMU2VydmVyRENMVmlzaXRvci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...phere/sql/parser/visitor/impl/SQL92DCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXNxbDkyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9TUUw5MkRDTFZpc2l0b3IuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...here/sql/parser/visitor/impl/OracleDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW9yYWNsZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci92aXNpdG9yL2ltcGwvT3JhY2xlRENMVmlzaXRvci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...ngjdbc/jdbc/adapter/AbstractConnectionAdapter.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmctamRiYy9zaGFyZGluZy1qZGJjLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5namRiYy9qZGJjL2FkYXB0ZXIvQWJzdHJhY3RDb25uZWN0aW9uQWRhcHRlci5qYXZh) | `73.52% <ø> (-0.26%)` | `6 <0> (ø)` | |
   | [.../segment/dml/item/ExpressionProjectionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kbWwvaXRlbS9FeHByZXNzaW9uUHJvamVjdGlvblNlZ21lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...tener/PostShardingRegistryCenterEventListener.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmctb3JjaGVzdHJhdGlvbi9zaGFyZGluZy1vcmNoZXN0cmF0aW9uLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL29yY2hlc3RyYXRpb24vaW50ZXJuYWwvcmVnaXN0cnkvbGlzdGVuZXIvUG9zdFNoYXJkaW5nUmVnaXN0cnlDZW50ZXJFdmVudExpc3RlbmVyLmphdmE=) | `42.85% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [.../sql/segment/dml/item/ColumnProjectionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kbWwvaXRlbS9Db2x1bW5Qcm9qZWN0aW9uU2VnbWVudC5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [.../sql/parser/visitor/impl/PostgreSQLDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXBvc3RncmVzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NxbC9wYXJzZXIvdmlzaXRvci9pbXBsL1Bvc3RncmVTUUxEQ0xWaXNpdG9yLmphdmE=) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...e/sql/parser/sql/segment/generic/TableSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9nZW5lcmljL1RhYmxlU2VnbWVudC5qYXZh) | `33.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | ... and [68 more](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=footer). Last update [a569f0e...81cd74e](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] codecov-io edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#issuecomment-591945952
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=h1) Report
   > Merging [#4498](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/a569f0e561bf1fc0fe52db31734e23e9c5ea4fdd?src=pr&el=desc) will **increase** coverage by `0.01%`.
   > The diff coverage is `14.8%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4498      +/-   ##
   ============================================
   + Coverage     55.23%   55.24%   +0.01%     
     Complexity      330      330              
   ============================================
     Files           953      953              
     Lines         18027    18026       -1     
     Branches       3408     3408              
   ============================================
   + Hits           9957     9959       +2     
   + Misses         7425     7422       -3     
     Partials        645      645
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...phere/sql/parser/visitor/impl/MySQLDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW15c3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9NeVNRTERDTFZpc2l0b3IuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...e/sql/parser/visitor/impl/SQLServerDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXNxbHNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci92aXNpdG9yL2ltcGwvU1FMU2VydmVyRENMVmlzaXRvci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...phere/sql/parser/visitor/impl/SQL92DCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXNxbDkyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9TUUw5MkRDTFZpc2l0b3IuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...here/sql/parser/visitor/impl/OracleDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW9yYWNsZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci92aXNpdG9yL2ltcGwvT3JhY2xlRENMVmlzaXRvci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...ngjdbc/jdbc/adapter/AbstractConnectionAdapter.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmctamRiYy9zaGFyZGluZy1qZGJjLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5namRiYy9qZGJjL2FkYXB0ZXIvQWJzdHJhY3RDb25uZWN0aW9uQWRhcHRlci5qYXZh) | `73.52% <ø> (-0.26%)` | `6 <0> (ø)` | |
   | [.../segment/dml/item/ExpressionProjectionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kbWwvaXRlbS9FeHByZXNzaW9uUHJvamVjdGlvblNlZ21lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...tener/PostShardingRegistryCenterEventListener.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmctb3JjaGVzdHJhdGlvbi9zaGFyZGluZy1vcmNoZXN0cmF0aW9uLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL29yY2hlc3RyYXRpb24vaW50ZXJuYWwvcmVnaXN0cnkvbGlzdGVuZXIvUG9zdFNoYXJkaW5nUmVnaXN0cnlDZW50ZXJFdmVudExpc3RlbmVyLmphdmE=) | `42.85% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [.../sql/segment/dml/item/ColumnProjectionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kbWwvaXRlbS9Db2x1bW5Qcm9qZWN0aW9uU2VnbWVudC5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [.../sql/parser/visitor/impl/PostgreSQLDCLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXBvc3RncmVzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NxbC9wYXJzZXIvdmlzaXRvci9pbXBsL1Bvc3RncmVTUUxEQ0xWaXNpdG9yLmphdmE=) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...e/sql/parser/sql/segment/generic/TableSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9nZW5lcmljL1RhYmxlU2VnbWVudC5qYXZh) | `33.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | ... and [68 more](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=footer). Last update [a569f0e...f4634be](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4498?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385492018
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +101,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertEquals(expectValue, actualValue[0]);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
+        assertEquals(null, nacosConfigCenterRepository.get("/sharding/nonExistentKey"));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWhenThrowException() {
+        doThrow(NacosException.class).when(configService).getConfig(eq("sharding.test"), eq(group), anyLong());
+        assertEquals(null, nacosConfigCenterRepository.get("/sharding/test"));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertUpdate() {
+        String updatedValue = "newValue";
+        nacosConfigCenterRepository.persist("/sharding/test", updatedValue);
+        verify(configService).publishConfig("sharding.test", group, updatedValue);
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertWatchUpdatedChangedType() {
+        final String expectValue = "expectValue";
+        final String[] actualValue = {null};
+        final DataChangedEvent.ChangedType[] actualType = {null};
+        doAnswer(AdditionalAnswers.answerVoid(getListenerAnswer(expectValue)))
+                .when(configService)
+                .addListener(anyString(), anyString(), any(Listener.class));
+        DataChangedEventListener listener = new DataChangedEventListener() {
+    
+            @Override
+            public void onChange(final DataChangedEvent dataChangedEvent) {
+                actualValue[0] = dataChangedEvent.getValue();
+                actualType[0] = dataChangedEvent.getChangedType();
+            }
+        };
+        nacosConfigCenterRepository.watch("/sharding/test", listener);
+        assertEquals(expectValue, actualValue[0]);
+        assertEquals(DataChangedEvent.ChangedType.UPDATED, actualType[0]);
 
 Review comment:
   Please use assertThat instead of assertEquals

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385564925
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -99,7 +102,69 @@ public void onChange(final DataChangedEvent dataChangedEvent) {
             }
         };
         nacosConfigCenterRepository.watch("/sharding/test", listener);
-        Assert.assertEquals(expectValue, actualValue[0]);
+        assertThat(expectValue, is(actualValue[0]));
+    }
+    
+    @Test
+    @SneakyThrows
+    public void assertGetWithNonExistentKey() {
+        when(configService.getConfig(eq("sharding.nonExistentKey"), eq(group), anyLong())).thenReturn(null);
 
 Review comment:
   Do not need mock if return null

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385103279
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -74,7 +76,7 @@ public void assertPersist() {
         nacosConfigCenterRepository.persist("/sharding/test", value);
         verify(configService).publishConfig("sharding.test", group, value);
     }
-    
+
 
 Review comment:
   This should keep blank character.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4498: Add Unit test case to NacosConfigInstanceTest
URL: https://github.com/apache/incubator-shardingsphere/pull/4498#discussion_r385565652
 
 

 ##########
 File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
 ##########
 @@ -80,7 +83,7 @@ public void assertPersist() {
     public void assertGet() {
         String value = "value";
         when(configService.getConfig(eq("sharding.test"), eq(group), anyLong())).thenReturn(value);
-        Assert.assertEquals(value, nacosConfigCenterRepository.get("/sharding/test"));
+        assertThat(value, is(nacosConfigCenterRepository.get("/sharding/test")));
 
 Review comment:
   The first arg of assertThat is `actual`, second arg is `expected`.
   It is reversed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services