vlsi commented on a change in pull request #2369:
URL: https://github.com/apache/calcite/pull/2369#discussion_r593056300
##########
File path: server/src/test/java/org/apache/calcite/test/ServerTest.java
##########
@@ -127,6 +127,16 @@ static Connection connect() throws SQLException {
assertThat(r.getInt(1), is(1));
assertThat(r.next(), is(false));
}
+
+ b = s.execute("create schema if not exists s");
+ assertThat(b, is(false));
Review comment:
> The introduced changes do not make the test more confusing
The introduced changes make it hard to understand the nature of the changes.
I do not ask you to refactor all the existing asserts in the test.
I just kindly ask you to use a better style for the newly added code.
The question from xy2953396112 highlights that it is not really clear what
`true/false` means in this context (it is tempting to assume that `true/false`
means success/failure which turns out to be wrong).
Thanks.
----------------------------------------------------------------
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:
[email protected]
