SNOW-1213120: Reuse connections in tests 3

[sfc-gh-ext-simba-jf]

Overview

SNOW-1213120

Pre-review self checklist

• PR branch is updated with all the changes from master branch
• The code is correctly formatted (run mvn -P check-style validate)
• New public API is not unnecessary exposed (run mvn verify and inspect target/japicmp/japicmp.html)
• The pull request name is prefixed with SNOW-XXXX:
• Code is in compliance with internal logging requirements

External contributors - please answer these questions before submitting a pull request. Thanks!

1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

   Fixes #NNNN

2. Fill out the following pre-review checklist:

   • I am adding a new automated test(s) to verify correctness of my new code
   • I am adding new logging messages
   • I am modifying authorization mechanisms
   • I am adding new credentials
   • I am modifying OCSP code
   • I am adding a new dependency or upgrading an existing one
   • I am adding new public/protected component not marked with @SnowflakeJdbcInternalApi (note that public/protected methods/fields in classes marked with this annotation are already internal)

3. Please describe how your code solves the related issue.

   Please write a short description of how your code change solves the related issue.

[sfc-gh-dprzybysz]

how can it be possible to have closed connection here? it should be renew by the base class before tests in this class?

[sfc-gh-ext-simba-jf]

I actually was wondering about this as well because when I ran this locally it never failed, but running in github it was constantly showing "connection closed" errors. When I added println statements to see where the connection was getting closed while running on github it looked like PreparedMultiStmtArrowIT (which extends PreparedMultiStmtIT and essentially just runs the same tests with arrow as the result format) would run first and then close that shared connection, and for some reason the "BeforeClass" from BaseJDBCWithSharedConnectionIT wasn't running again for the second test suite.

[sfc-gh-dprzybysz]

Indeed it may happen for parameterized tests
then we can organize it in a different way in tests and have e.g. connections with specific result format set upfront

e.g. for parameterized tests we can recreate the connection in the constructor when necessary and it may involve not using the same base class that you have created before

I addressed it in the not merged PR https://github.com/snowflakedb/snowflake-jdbc/pull/1754/files#diff-b01b6456eab10190c84533701ca0d1a477f5463c060522920c63fa25ab6fc765, you can take an inspiration from this PR

[sfc-gh-ext-simba-jf]

It's not actually running as a true parameterized test though. PreparedMultiStmtArrowIT simply extends PreparedMultiStmtIT and calls the super constructor passing in the "arrow" query format (

snowflake-jdbc/src/test/java/net/snowflake/client/jdbc/PreparedMultiStmtArrowIT.java

Line 7 in 1d5b695

public class PreparedMultiStmtArrowIT extends PreparedMultiStmtIT {

)

I'm thinking if we want to change this, it might be better to simplify and refactor PreparedMultiStmtIT to actually be parameterized and run both json and arrow tests and to delete PreparedMultiStmtArrowIT, rather than to introduce an additional base class for the connection sharing. Unless there is some reason we want to keep the arrow tests as a distinct class?

[sfc-gh-dprzybysz]

it's a great idea to go fully with parameterized tests. The reason why it's split may be will to use TestCategoryArrow, but it should not be the blocker - please rewrite the tests here.
The same case applies in MultiStatementArrowIT, ResultSetArrowIT, ResultSetArrowLatestIT, PreparedStatementArrow1IT, PreparedStatementArrow1LatestIT, PreparedStatementArrow2IT, StatementArrowIT

Based on that it is possible that we will need to think about another categories and it will affect distribution in #1735

[sfc-gh-pbulawa]

Could we execute it once, in @BeforeClass instead of setting it for every test?