sandynz commented on a change in pull request #15803:
URL: https://github.com/apache/shardingsphere/pull/15803#discussion_r820071160
##########
File path:
shardingsphere-test/shardingsphere-pipeline-test/src/test/java/org/apache/shardingsphere/data/pipeline/core/metadata/model/PipelineTableMetaDataTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.shardingsphere.data.pipeline.core.metadata.model;
+
Review comment:
License header should be added at the beginning of java code. You could
refer to other source code.
And it's better to run unit test before submiting PR. How to do it?
1. In terminal, cd to `shardingsphere-test/shardingsphere-pipeline-test`
directory (assume it's in shardingsphere project root)
2. Execute command: `mvn clean install -Dmaven.javadoc.skip=true -Prelease`,
fix error and run it again until it pass.
##########
File path:
shardingsphere-test/shardingsphere-pipeline-test/src/test/java/org/apache/shardingsphere/data/pipeline/core/metadata/model/PipelineTableMetaDataTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.shardingsphere.data.pipeline.core.metadata.model;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.Types;
+import java.util.List;
+import java.util.Map;
+
+public final class PipelineTableMetaDataTest {
+ private PipelineTableMetaData pipelineTableMetaData;
+ private Map<String, PipelineColumnMetaData> columnMetaDataMap;
+
+ @Before
+ public void setUp() {
+ PipelineColumnMetaData pipelineColumnMetaData = new
PipelineColumnMetaData(0, "test", Types.INTEGER, "INTEGER", true);
+ columnMetaDataMap.put("test_column", pipelineColumnMetaData);
+ pipelineTableMetaData = new PipelineTableMetaData(null,
columnMetaDataMap);
+ }
+
+ @Test
+ public void assertGetColumnMetaDataGivenColumnIndex() {
+ List<String> pipelineTableColumns =
pipelineTableMetaData.getColumnNames();
+ PipelineColumnMetaData actual =
columnMetaDataMap.get(pipelineTableColumns.get(0));
+ assertThat(actual.getOrdinalPosition(), is(0));
Review comment:
1. It's better to invoke `pipelineTableMetaData.getColumnMetaData`
instead of `columnMetaDataMap.get(pipelineTableColumns.get(0))`, since the
logic is encapsulated in `pipelineTableMetaData` and we'll test it.
2. `columnMetaDataMap` could be hidden in `setUp`.
##########
File path:
shardingsphere-test/shardingsphere-pipeline-test/src/test/java/org/apache/shardingsphere/data/pipeline/core/metadata/model/PipelineTableMetaDataTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.shardingsphere.data.pipeline.core.metadata.model;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.Types;
+import java.util.List;
+import java.util.Map;
+
+public final class PipelineTableMetaDataTest {
+ private PipelineTableMetaData pipelineTableMetaData;
+ private Map<String, PipelineColumnMetaData> columnMetaDataMap;
+
+ @Before
+ public void setUp() {
+ PipelineColumnMetaData pipelineColumnMetaData = new
PipelineColumnMetaData(0, "test", Types.INTEGER, "INTEGER", true);
+ columnMetaDataMap.put("test_column", pipelineColumnMetaData);
+ pipelineTableMetaData = new PipelineTableMetaData(null,
columnMetaDataMap);
+ }
+
+ @Test
+ public void assertGetColumnMetaDataGivenColumnIndex() {
+ List<String> pipelineTableColumns =
pipelineTableMetaData.getColumnNames();
+ PipelineColumnMetaData actual =
columnMetaDataMap.get(pipelineTableColumns.get(0));
+ assertThat(actual.getOrdinalPosition(), is(0));
+ assertThat(actual.getName(), is("test"));
+ assertThat(actual.getDataType(), is(Types.INTEGER));
+ assertTrue(actual.isPrimaryKey());
+ }
+
+ @Test
+ public void assertGetColumnMetaDataGivenColumnName() {
+ PipelineColumnMetaData actual = columnMetaDataMap.get("test_column");
+ assertThat(actual.getOrdinalPosition(), is(0));
+ assertThat(actual.getName(), is("test"));
+ assertThat(actual.getDataType(), is(Types.INTEGER));
+ assertTrue(actual.isPrimaryKey());
+ }
Review comment:
1. It's better to invoke `pipelineTableMetaData.getColumnMetaData`
instead of `columnMetaDataMap.get("test_column")`, since the logic is
encapsulated in `pipelineTableMetaData` and we'll test it.
2. It could be better to add a new case: get column metadata by
non-existence column name.
##########
File path:
shardingsphere-test/shardingsphere-pipeline-test/src/test/java/org/apache/shardingsphere/data/pipeline/core/metadata/model/PipelineTableMetaDataTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.shardingsphere.data.pipeline.core.metadata.model;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.Types;
+import java.util.List;
+import java.util.Map;
+
+public final class PipelineTableMetaDataTest {
+ private PipelineTableMetaData pipelineTableMetaData;
Review comment:
From code of conduct: `Keep one blank line after class definition.`
See [Code of Conduct](
https://shardingsphere.apache.org/community/en/contribute/code-conduct/ ) for
more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
