You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/09/08 05:48:59 UTC

[GitHub] [shardingsphere] zhaojinchao95 opened a new pull request, #20873: Refactor DistSQL RAL assertion logic

zhaojinchao95 opened a new pull request, #20873:
URL: https://github.com/apache/shardingsphere/pull/20873

   For #20867 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] LeeGuoPing commented on a diff in pull request #20873: Refactor DistSQL RAL assertion logic of integration test

Posted by GitBox <gi...@apache.org>.
LeeGuoPing commented on code in PR #20873:
URL: https://github.com/apache/shardingsphere/pull/20873#discussion_r965571058


##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-suite/src/test/java/org/apache/shardingsphere/test/integration/engine/ral/BaseRALIT.java:
##########
@@ -106,23 +115,27 @@ private void assertMetaData(final ResultSetMetaData actual, final Collection<Dat
         }
     }
     
-    private void assertRows(final ResultSet actual, final List<DataSetRow> expected) throws SQLException {
+    private void assertRows(final ResultSet actual, final Collection<String> notAssertionColumns, final List<DataSetRow> expected) throws SQLException {
         int rowCount = 0;
         ResultSetMetaData actualMetaData = actual.getMetaData();
         while (actual.next()) {
             assertTrue("Size of actual result set is different with size of expected dat set rows.", rowCount < expected.size());
-            assertRow(actual, actualMetaData, expected.get(rowCount));
+            assertRow(actual, notAssertionColumns, actualMetaData, expected.get(rowCount));
             rowCount++;
         }
         assertThat("Size of actual result set is different with size of expected dat set rows.", rowCount, is(expected.size()));
     }
     
-    private void assertRow(final ResultSet actual, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
+    private void assertRow(final ResultSet actual, final Collection<String> notAssertionColumns, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
         int columnIndex = 1;
         for (String each : expected.splitValues("|")) {
             String columnLabel = actualMetaData.getColumnLabel(columnIndex);
-            assertObjectValue(actual, columnIndex, columnLabel, each);
-            columnIndex++;
+            if (notAssertionColumns.contains(columnLabel)) {

Review Comment:
   `Suggest `is a verb,use `suggestion` here may better. Do you think so?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] LeeGuoPing commented on a diff in pull request #20873: Refactor DistSQL RAL assertion logic of integration test

Posted by GitBox <gi...@apache.org>.
LeeGuoPing commented on code in PR #20873:
URL: https://github.com/apache/shardingsphere/pull/20873#discussion_r965563476


##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-suite/src/test/java/org/apache/shardingsphere/test/integration/engine/ral/BaseRALIT.java:
##########
@@ -106,23 +115,27 @@ private void assertMetaData(final ResultSetMetaData actual, final Collection<Dat
         }
     }
     
-    private void assertRows(final ResultSet actual, final List<DataSetRow> expected) throws SQLException {
+    private void assertRows(final ResultSet actual, final Collection<String> notAssertionColumns, final List<DataSetRow> expected) throws SQLException {
         int rowCount = 0;
         ResultSetMetaData actualMetaData = actual.getMetaData();
         while (actual.next()) {
             assertTrue("Size of actual result set is different with size of expected dat set rows.", rowCount < expected.size());
-            assertRow(actual, actualMetaData, expected.get(rowCount));
+            assertRow(actual, notAssertionColumns, actualMetaData, expected.get(rowCount));
             rowCount++;
         }
         assertThat("Size of actual result set is different with size of expected dat set rows.", rowCount, is(expected.size()));
     }
     
-    private void assertRow(final ResultSet actual, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
+    private void assertRow(final ResultSet actual, final Collection<String> notAssertionColumns, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
         int columnIndex = 1;
         for (String each : expected.splitValues("|")) {
             String columnLabel = actualMetaData.getColumnLabel(columnIndex);
-            assertObjectValue(actual, columnIndex, columnLabel, each);
-            columnIndex++;
+            if (notAssertionColumns.contains(columnLabel)) {

Review Comment:
           if (!notAssertionColumns.contains(columnLabel)) {
               assertObjectValue(actual, columnIndex, columnLabel, each);
           }
           columnIndex++;



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] zhaojinchao95 commented on pull request #20873: Refactor DistSQL RAL assertion logic of integration test

Posted by GitBox <gi...@apache.org>.
zhaojinchao95 commented on PR #20873:
URL: https://github.com/apache/shardingsphere/pull/20873#issuecomment-1240321968

   <img width="1905" alt="image" src="https://user-images.githubusercontent.com/33742097/189059053-06c99b8b-2cd8-4b44-8344-a80ba54ed59f.png">
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] LeeGuoPing merged pull request #20873: Refactor DistSQL RAL assertion logic of integration test

Posted by GitBox <gi...@apache.org>.
LeeGuoPing merged PR #20873:
URL: https://github.com/apache/shardingsphere/pull/20873


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] zhaojinchao95 commented on a diff in pull request #20873: Refactor DistSQL RAL assertion logic of integration test

Posted by GitBox <gi...@apache.org>.
zhaojinchao95 commented on code in PR #20873:
URL: https://github.com/apache/shardingsphere/pull/20873#discussion_r965567005


##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-suite/src/test/java/org/apache/shardingsphere/test/integration/engine/ral/BaseRALIT.java:
##########
@@ -106,23 +115,27 @@ private void assertMetaData(final ResultSetMetaData actual, final Collection<Dat
         }
     }
     
-    private void assertRows(final ResultSet actual, final List<DataSetRow> expected) throws SQLException {
+    private void assertRows(final ResultSet actual, final Collection<String> notAssertionColumns, final List<DataSetRow> expected) throws SQLException {
         int rowCount = 0;
         ResultSetMetaData actualMetaData = actual.getMetaData();
         while (actual.next()) {
             assertTrue("Size of actual result set is different with size of expected dat set rows.", rowCount < expected.size());
-            assertRow(actual, actualMetaData, expected.get(rowCount));
+            assertRow(actual, notAssertionColumns, actualMetaData, expected.get(rowCount));
             rowCount++;
         }
         assertThat("Size of actual result set is different with size of expected dat set rows.", rowCount, is(expected.size()));
     }
     
-    private void assertRow(final ResultSet actual, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
+    private void assertRow(final ResultSet actual, final Collection<String> notAssertionColumns, final ResultSetMetaData actualMetaData, final DataSetRow expected) throws SQLException {
         int columnIndex = 1;
         for (String each : expected.splitValues("|")) {
             String columnLabel = actualMetaData.getColumnLabel(columnIndex);
-            assertObjectValue(actual, columnIndex, columnLabel, each);
-            columnIndex++;
+            if (notAssertionColumns.contains(columnLabel)) {

Review Comment:
   Thx, i think your suggest are right.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org