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/08/08 07:11:00 UTC

[GitHub] [shardingsphere] cowcomic opened a new pull request #6701: add ProxyDataSourceContextTest

cowcomic opened a new pull request #6701:
URL: https://github.com/apache/shardingsphere/pull/6701


   Fixes #6568.
   
   Changes proposed in this pull request:
   add test
   -
   -
   -
   


----------------------------------------------------------------
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



[GitHub] [shardingsphere] terrymanu commented on a change in pull request #6701: add ProxyDataSourceContextTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #6701:
URL: https://github.com/apache/shardingsphere/pull/6701#discussion_r467464664



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().isEmpty());
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertWrongSchemaDataSources() {
+        // schameName -- dataSourceName -- dataSource
+        DataSourceParameter dataSourceParameter = new DataSourceParameter();
+        dataSourceParameter.setUrl("jdbc11:mysql11:xxx");
+        Map<String, DataSourceParameter> dataSourceParameterMap = new LinkedHashMap<>();
+        dataSourceParameterMap.put("order1", dataSourceParameter);
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        schemaDataSources.put("order", dataSourceParameterMap);
+        new ProxyDataSourceContext(schemaDataSources);
+        //assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertThrowByBuild() throws Exception {
+        // schameName -- dataSourceName -- dataSource

Review comment:
       Please remove useless comment.
   Include Javadoc, todo and fixme only in the comments.
   FYI: https://shardingsphere.apache.org/community/en/contribute/code-conduct/

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().isEmpty());
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertWrongSchemaDataSources() {
+        // schameName -- dataSourceName -- dataSource
+        DataSourceParameter dataSourceParameter = new DataSourceParameter();
+        dataSourceParameter.setUrl("jdbc11:mysql11:xxx");
+        Map<String, DataSourceParameter> dataSourceParameterMap = new LinkedHashMap<>();
+        dataSourceParameterMap.put("order1", dataSourceParameter);
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        schemaDataSources.put("order", dataSourceParameterMap);
+        new ProxyDataSourceContext(schemaDataSources);
+        //assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertThrowByBuild() throws Exception {
+        // schameName -- dataSourceName -- dataSource
+        JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory = Mockito.mock(JDBCRawBackendDataSourceFactory.class);
+        Mockito.when(jdbcRawBackendDataSourceFactory.build(Mockito.anyString(), Mockito.any())).thenThrow(new ShardingSphereException(""));

Review comment:
       Please use static import for Mockito

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {

Review comment:
       Please add final if the class is not design for extension

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().isEmpty());
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertWrongSchemaDataSources() {
+        // schameName -- dataSourceName -- dataSource
+        DataSourceParameter dataSourceParameter = new DataSourceParameter();
+        dataSourceParameter.setUrl("jdbc11:mysql11:xxx");
+        Map<String, DataSourceParameter> dataSourceParameterMap = new LinkedHashMap<>();
+        dataSourceParameterMap.put("order1", dataSourceParameter);
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        schemaDataSources.put("order", dataSourceParameterMap);
+        new ProxyDataSourceContext(schemaDataSources);
+        //assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);

Review comment:
       Please do not leave the comment codes

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().isEmpty());
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertWrongSchemaDataSources() {
+        // schameName -- dataSourceName -- dataSource
+        DataSourceParameter dataSourceParameter = new DataSourceParameter();
+        dataSourceParameter.setUrl("jdbc11:mysql11:xxx");
+        Map<String, DataSourceParameter> dataSourceParameterMap = new LinkedHashMap<>();
+        dataSourceParameterMap.put("order1", dataSourceParameter);
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        schemaDataSources.put("order", dataSourceParameterMap);
+        new ProxyDataSourceContext(schemaDataSources);
+        //assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertThrowByBuild() throws Exception {
+        // schameName -- dataSourceName -- dataSource
+        JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory = Mockito.mock(JDBCRawBackendDataSourceFactory.class);
+        Mockito.when(jdbcRawBackendDataSourceFactory.build(Mockito.anyString(), Mockito.any())).thenThrow(new ShardingSphereException(""));
+        build(jdbcRawBackendDataSourceFactory);
+        Mockito.reset(jdbcRawBackendDataSourceFactory);
+    }
+
+    @Test
+    public void assertRightMysqlSchemaDataSources() throws Exception {
+        // schameName -- dataSourceName -- dataSource
+        JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory = Mockito.mock(JDBCRawBackendDataSourceFactory.class);
+        Mockito.when(jdbcRawBackendDataSourceFactory.build(Mockito.anyString(), Mockito.any())).thenReturn(new HikariDataSource());
+        ProxyDataSourceContext proxyDataSourceContext = build(jdbcRawBackendDataSourceFactory);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().size() == 1);
+        Mockito.reset(jdbcRawBackendDataSourceFactory);
+    }
+
+    private ProxyDataSourceContext build(final JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory) throws Exception {
+        JDBCRawBackendDataSourceFactory jdbcBackendDataSourceFactory = (JDBCRawBackendDataSourceFactory) JDBCRawBackendDataSourceFactory.getInstance();
+        Class<?> jdbcBackendDataSourceFactoryClass = jdbcBackendDataSourceFactory.getClass();
+        Field field = jdbcBackendDataSourceFactoryClass.getDeclaredField("INSTANCE");
+        Field modifiers = field.getClass().getDeclaredField("modifiers");
+        modifiers.setAccessible(true);
+        modifiers.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+        field.setAccessible(true);
+        field.set(field, jdbcRawBackendDataSourceFactory);
+

Review comment:
       Please remove useless blank line

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);

Review comment:
       Please use assertThat and InstanceOf to assert

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/schema/ProxyDataSourceContextTest.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.proxy.backend.schema;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.kernel.context.schema.DataSourceParameter;
+import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCRawBackendDataSourceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class ProxyDataSourceContextTest {
+
+    @Test
+    public void assertEmptySchemaDataSources() {
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        ProxyDataSourceContext proxyDataSourceContext = new ProxyDataSourceContext(schemaDataSources);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+        assertTrue(proxyDataSourceContext.getDataSourcesMap().isEmpty());
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertWrongSchemaDataSources() {
+        // schameName -- dataSourceName -- dataSource
+        DataSourceParameter dataSourceParameter = new DataSourceParameter();
+        dataSourceParameter.setUrl("jdbc11:mysql11:xxx");
+        Map<String, DataSourceParameter> dataSourceParameterMap = new LinkedHashMap<>();
+        dataSourceParameterMap.put("order1", dataSourceParameter);
+        Map<String, Map<String, DataSourceParameter>> schemaDataSources = new HashMap<>();
+        schemaDataSources.put("order", dataSourceParameterMap);
+        new ProxyDataSourceContext(schemaDataSources);
+        //assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);
+    }
+
+    @Test(expected = ShardingSphereException.class)
+    public void assertThrowByBuild() throws Exception {
+        // schameName -- dataSourceName -- dataSource
+        JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory = Mockito.mock(JDBCRawBackendDataSourceFactory.class);
+        Mockito.when(jdbcRawBackendDataSourceFactory.build(Mockito.anyString(), Mockito.any())).thenThrow(new ShardingSphereException(""));
+        build(jdbcRawBackendDataSourceFactory);
+        Mockito.reset(jdbcRawBackendDataSourceFactory);
+    }
+
+    @Test
+    public void assertRightMysqlSchemaDataSources() throws Exception {
+        // schameName -- dataSourceName -- dataSource
+        JDBCRawBackendDataSourceFactory jdbcRawBackendDataSourceFactory = Mockito.mock(JDBCRawBackendDataSourceFactory.class);
+        Mockito.when(jdbcRawBackendDataSourceFactory.build(Mockito.anyString(), Mockito.any())).thenReturn(new HikariDataSource());
+        ProxyDataSourceContext proxyDataSourceContext = build(jdbcRawBackendDataSourceFactory);
+        assertTrue(proxyDataSourceContext.getDatabaseType() instanceof MySQLDatabaseType);

Review comment:
       Please use assertThat and InstanceOf to assert




----------------------------------------------------------------
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



[GitHub] [shardingsphere] terrymanu commented on a change in pull request #6701: add ProxyDataSourceContextTest

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #6701:
URL: https://github.com/apache/shardingsphere/pull/6701#discussion_r467419926



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/pom.xml
##########
@@ -132,5 +132,18 @@
             <artifactId>postgresql</artifactId>
             <scope>runtime</scope>
         </dependency>
+
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-module-junit4</artifactId>
+            <version>1.7.4</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito2</artifactId>
+            <version>1.7.4</version>
+            <scope>test</scope>
+        </dependency>

Review comment:
       Please do not add other dependency if unnecessary, the reason are:
   1. It is better to use one mock framework
   2. If we need add more mock framework, please add the notice and licenses too




----------------------------------------------------------------
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



[GitHub] [shardingsphere] codecov-commenter commented on pull request #6701: add ProxyDataSourceContextTest

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6701:
URL: https://github.com/apache/shardingsphere/pull/6701#issuecomment-670924634


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/6701?src=pr&el=h1) Report
   > Merging [#6701](https://codecov.io/gh/apache/shardingsphere/pull/6701?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere/commit/77b28096b8e96b989c240e86521c281439c16b50&el=desc) will **increase** coverage by `0.72%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/6701/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so)](https://codecov.io/gh/apache/shardingsphere/pull/6701?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6701      +/-   ##
   ============================================
   + Coverage     53.24%   53.97%   +0.72%     
   + Complexity      428      427       -1     
   ============================================
     Files          1236     1237       +1     
     Lines         22011    21975      -36     
     Branches       3952     3956       +4     
   ============================================
   + Hits          11720    11860     +140     
   + Misses         9530     9343     -187     
   - Partials        761      772      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/6701?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../infra/yaml/config/YamlRootRuleConfigurations.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS95YW1sL2NvbmZpZy9ZYW1sUm9vdFJ1bGVDb25maWd1cmF0aW9ucy5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...config/rule/YamlEncryptTableRuleConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL2NvbmZpZy9ydWxlL1lhbWxFbmNyeXB0VGFibGVSdWxlQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...onfig/rule/YamlEncryptColumnRuleConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL2NvbmZpZy9ydWxlL1lhbWxFbmNyeXB0Q29sdW1uUnVsZUNvbmZpZ3VyYXRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | [.../representer/processor/NoneYamlTupleProcessor.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFyZGluZy95YW1sL2VuZ2luZS9yZXByZXNlbnRlci9wcm9jZXNzb3IvTm9uZVlhbWxUdXBsZVByb2Nlc3Nvci5qYXZh) | `0.00% <0.00%> (-80.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | [...ule/EncryptColumnRuleConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL3N3YXBwZXIvcnVsZS9FbmNyeXB0Q29sdW1uUnVsZUNvbmZpZ3VyYXRpb25ZYW1sU3dhcHBlci5qYXZh) | `0.00% <0.00%> (-77.78%)` | `0.00% <0.00%> (ø%)` | |
   | [...rypt/yaml/config/YamlEncryptRuleConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL2NvbmZpZy9ZYW1sRW5jcnlwdFJ1bGVDb25maWd1cmF0aW9uLmphdmE=) | `0.00% <0.00%> (-75.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...rule/EncryptTableRuleConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL3N3YXBwZXIvcnVsZS9FbmNyeXB0VGFibGVSdWxlQ29uZmlndXJhdGlvbllhbWxTd2FwcGVyLmphdmE=) | `0.00% <0.00%> (-50.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...l/swapper/EncryptRuleConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL3N3YXBwZXIvRW5jcnlwdFJ1bGVDb25maWd1cmF0aW9uWWFtbFN3YXBwZXIuamF2YQ==) | `0.00% <0.00%> (-41.67%)` | `0.00% <0.00%> (ø%)` | |
   | [...RuleAlgorithmProviderConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC95YW1sL3N3YXBwZXIvRW5jcnlwdFJ1bGVBbGdvcml0aG1Qcm92aWRlckNvbmZpZ3VyYXRpb25ZYW1sU3dhcHBlci5qYXZh) | `0.00% <0.00%> (-26.32%)` | `0.00% <0.00%> (ø%)` | |
   | [...ml/swapper/ShadowRuleConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhZG93L3NoYXJkaW5nc3BoZXJlLXNoYWRvdy1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYWRvdy95YW1sL3N3YXBwZXIvU2hhZG93UnVsZUNvbmZpZ3VyYXRpb25ZYW1sU3dhcHBlci5qYXZh) | `66.66% <0.00%> (-22.23%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [161 more](https://codecov.io/gh/apache/shardingsphere/pull/6701/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/6701?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/shardingsphere/pull/6701?src=pr&el=footer). Last update [77b2809...c59ce45](https://codecov.io/gh/apache/shardingsphere/pull/6701?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



[GitHub] [shardingsphere] terrymanu merged pull request #6701: add ProxyDataSourceContextTest

Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #6701:
URL: https://github.com/apache/shardingsphere/pull/6701


   


----------------------------------------------------------------
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