add tests for call statements with and w/o preceding comment

[heimburgerj2]

test without comment currently succeeds while tests with preceding comment fail at the moment

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Test submission

• This PR adds some tests illustrating the bug described in Preceding comments in SQL CALL statements prevent setting of OUT parameters for stored procedures / functions #2538

[vlsi]

Thanks for adding some tests 👍

I see that the failure looks like

FAILURE   0.0sec, org.postgresql.core.ParserTest > testModifyJdbcCall
    java.lang.AssertionError: expected:<true> but was:<false>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:120)
at org.junit.Assert.assertEquals(Assert.java:146)
at org.postgresql.core.ParserTest.testModifyJdbcCall(ParserTest.java:160)

Could you please adjust the test code in such a way that the failure message looks like a decent bug report? (see https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md#test )

[heimburgerj2]

Could you please adjust the test code in such a way that the failure message looks like a decent bug report? (see https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md#test )

I have updated the tests accordingly. Please check whether this is sufficient.

[davecramer]

It would be clearer to me to say Parse error or Unable to parse.

[heimburgerj2]

updated the message

[vlsi]

do you think you could push Parser.modifyJdbcCall into assertIsFunction as well, so its failure message would indicate the way it called parsing?

[heimburgerj2]

I moved the call to assertIsFunction, which is a good idea since it reduces duplicate code. However, I don't see how this would improve the failure message.
Do you mind if I switch this test class ParserTest to JUnit5 instead of JUnit4?
Then we could use a @ParemeterizedTest annotated test method here instead of 4 test methods. This would also allow to add more parameters easily, e.g. the server version, though I doubt that this would impact this issue.

[vlsi]

which is a good idea since it reduces duplicate code

Glad you liked it. Every time I ask I am afraid it would be considered splitting hairs.

However, I don't see how this would improve the failure message.

How about the following?

    JdbcCallParseInfo jdbcCallParseInfo = Parser.modifyJdbcCall(sql, true, ServerVersion.v14.getVersionNum(), 3, EscapeSyntaxCallMode.CALL);
    String message = "Parser.modifyJdbcCall(\"" + sql + "\", , true, ServerVersion.v14.getVersionNum(), 3, EscapeSyntaxCallMode.CALL).isFunction was supposed to return FUNCTION, however got " + jdbcCallParseInfo;

---

Switch to JUnit5 is just fine.
Frankly speaking, I would love to rewrite the existing BaseTest4 into something like JUnit5 extension, however, I had no chances to do that yet.

[heimburgerj2]

JdbcCallParseInfo does not implement toString() obviously:

Parser.modifyJdbcCall("/* some comment */ { ? = call test_function(?)}", , true, ServerVersion.v14.getVersionNum(), 3, EscapeSyntaxCallMode.CALL).isFunction was supposed to return FUNCTION, however got org.postgresql.core.JdbcCallParseInfo@5f1b1944 expected:<true> but was:<false>

Therefore I omitted the however ... part:

Parser.modifyJdbcCall("/* some comment */ { ? = call test_function(?)}", , true, ServerVersion.v14.getVersionNum(), 3, EscapeSyntaxCallMode.CALL).isFunction was supposed to return FUNCTION,  expected:<true> but was:<false>

[heimburgerj2]

Switch to JUnit5 is just fine. Frankly speaking, I would love to rewrite the existing BaseTest4 into something like JUnit5 extension, however, I had no chances to do that yet.

I have converted the ParserTest to JUnit5 (update imports, swap method params) and changed the four test methods to one ParameterizedTest.