Refactor test setup/teardown to reuse connection across tests#155
Open
justing-bq wants to merge 2 commits intoapache-odbcfrom
Open
Refactor test setup/teardown to reuse connection across tests#155justing-bq wants to merge 2 commits intoapache-odbcfrom
justing-bq wants to merge 2 commits intoapache-odbcfrom
Conversation
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
2 times, most recently
from
183fa4d to
2922812
Compare
justing-bq
changed the title
justing-bq
force-pushed
the
apache-odbc
branch
from
2336a2d to
fcb22f1
Compare
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
from
2922812 to
1f59664
Compare
justing-bq
changed the title
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
from
1f59664 to
e740c26
Compare
alinaliBQ
reviewed
Feb 23, 2026
Comment on lines
428
to
430
| ulimit -c unlimited # must enable within the same shell | ||
| ci/scripts/cpp_test.sh $(pwd) $(pwd)/build | ||
| sudo ci/scripts/cpp_test.sh $(pwd) $(pwd)/build | ||
|
|
There was a problem hiding this comment.
I see the ODBC tests are passing on macOS CI which is awesome! This error is interesting:
Could not find a package configuration file provided by "Arrow" with any of
the following names:
ArrowConfig.cmake
arrow-config.cmake
From run: https://github.com/Bit-Quill/arrow/actions/runs/22157630099/job/64065881526?pr=155
There was a problem hiding this comment.
seems this line CMAKE_PREFIX_PATH= empty is the reason why the run fails.
|
As discussed, let's work on the |
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
2 times, most recently
from
ef007f3 to
8dd4bcf
Compare
alinaliBQ
reviewed
Feb 26, 2026
| @@ -41,78 +41,76 @@ | |||
| static constexpr std::string_view kTestConnectStr = "ARROW_FLIGHT_SQL_ODBC_CONN"; | |||
There was a problem hiding this comment.
Let's try setting the dsn into user DSN to avoid the sudo issue
alinaliBQ
reviewed
Feb 26, 2026
There was a problem hiding this comment.
Another option is to try CMAKE_PREFIX_PATH: /tmp/local with sudo on
alinaliBQ
force-pushed
the
apache-odbc
branch
from
0a3cd02 to
7825011
Compare
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
from
8dd4bcf to
227afb8
Compare
justing-bq
force-pushed
the
mockTestSetupTeardown
branch
from
227afb8 to
c9212b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Possible workaround to apache#49207 because it involves fewer setups/teardowns.
What changes are included in this PR?
Our mock test Setup/Teardown logic now happens once at the environment level.
Our test connect/disconnect now happens once per test fixture instead of once per test.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.