You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2020/09/01 11:43:05 UTC
[GitHub] [incubator-iotdb] wyh-ztf opened a new pull request #1670: IOTDB-856:Clear up the IoTDBSessionIT
wyh-ztf opened a new pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670
----------------------------------------------------------------
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] [incubator-iotdb] qiaojialin commented on pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
qiaojialin commented on pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#issuecomment-686411359
Hi, please fix the conflict~
----------------------------------------------------------------
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] [incubator-iotdb] yhwang-hbl commented on a change in pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
yhwang-hbl commented on a change in pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#discussion_r483334556
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionSimpleIT.java
##########
@@ -0,0 +1,243 @@
+package org.apache.iotdb.session;
Review comment:
> Apache Rat, please.
Copy that,thanks for your guidance
----------------------------------------------------------------
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] [incubator-iotdb] wyh-ztf commented on pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
wyh-ztf commented on pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#issuecomment-686192125
> It looks better 👍 One more step, we could move some tests that are independent to another IT file
>
> There will be two IT files:
> IoTDBSessionComplexIT
> IoTDBSessionSimpleIT
OK, thanks for your suggestion. I will improve 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
[GitHub] [incubator-iotdb] jixuan1989 commented on a change in pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#discussion_r483327126
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionSimpleIT.java
##########
@@ -0,0 +1,243 @@
+package org.apache.iotdb.session;
Review comment:
Apache Rat, please.
----------------------------------------------------------------
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] [incubator-iotdb] yhwang-hbl commented on pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
yhwang-hbl commented on pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#issuecomment-686461648
> Hi, please fix the conflict~
Copy that
----------------------------------------------------------------
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] [incubator-iotdb] qiaojialin merged pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
qiaojialin merged pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670
----------------------------------------------------------------
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] [incubator-iotdb] qiaojialin commented on a change in pull request #1670: [IOTDB-856] Clear up the IoTDBSessionIT
Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #1670:
URL: https://github.com/apache/incubator-iotdb/pull/1670#discussion_r481135639
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##########
@@ -244,23 +241,25 @@ public void testAlignByDevice() throws IoTDBConnectionException,
createTimeseries();
- insertTabletTest2("root.sg1.d1");
+ insertTabletTest("root.sg1.d1");
queryForAlignByDevice();
- queryForAlignByDevice2();
+
+ session.close();
}
- // it's will output too much to travis, so ignore it
public void testTime()
Review comment:
This method could be deleted
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##########
@@ -814,7 +831,7 @@ private void insert() throws IoTDBConnectionException, StatementExecutionExcepti
}
}
- private void insertTabletTest1(String deviceId)
+ private void insertTabletTest(String deviceId)
Review comment:
this is not a test, it's just a tool. Renaming to insertTablet is ok
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##########
@@ -88,9 +88,8 @@ public void testInsertByStr() throws IoTDBConnectionException, StatementExecutio
createTimeseries();
insertByStr();
- // sql test
insert_via_sql();
Review comment:
rename this to insertViaSQL
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##########
@@ -1289,8 +1091,7 @@ private void queryForBatch() throws ClassNotFoundException, SQLException {
}
}
Assert.assertEquals(standard, resultStr.toString());
- // d1 and d2 will align
- Assert.assertEquals(14000, count);
+ Assert.assertEquals(700, count);
}
}
Review comment:
if the queryForBatchCheckOrder is not used, it could be removed.
##########
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##########
@@ -794,7 +811,7 @@ private void insertInObject() throws IoTDBConnectionException, StatementExecutio
}
}
- private void insert() throws IoTDBConnectionException, StatementExecutionException {
+ private void insertInObjectList() throws IoTDBConnectionException, StatementExecutionException {
String deviceId = "root.sg1.d1";
List<String> measurements = new ArrayList<>();
Review comment:
move the insertInObject() method to its caller. There are too many methods in this file..
----------------------------------------------------------------
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