Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split common tests into a common library #1174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

vadz
Copy link
Member

@vadz vadz commented Oct 22, 2024

Avoid recompiling common-tests.h many, many times.

This is the only place where they're used so far and it doesn't make
sense to compile them many times over when building the tests for the
other backends.

If these helpers are needed in the other backends tests, we should
extract them into their own header and include it as necessary.
It doesn't seem useful to use GLOB without any widlcards, so replace it
with just set().
This class is only used there, so don't define it in the common header.
This class seems to be completely useless, it's used by function_creator
in PostgreSQL test only, so just inline it there.
@vadz vadz force-pushed the split-tests branch 2 times, most recently from 50fd0a8 to c0ef0bc Compare October 23, 2024 00:50
This allows to compile them once, instead of doing it for every backend:
while this doesn't matter for the CI builds, recompiling common-tests.h
a dozen times enormously slowed down local builds using all backends.

Now it is compiled only once, as test-common.cpp, and all the other
tests (except for the "empty" one) just link with the resulting library.

Also extract some parts of this file into separate headers, that can be
included only by the tests that actually need them.

Note that the entire test-common.cpp probably ought to be split into
multiple files, to speed up its build too, but this can be done later.
@vadz vadz marked this pull request as ready for review October 23, 2024 13:14
@vadz
Copy link
Member Author

vadz commented Oct 23, 2024

@Krzmbrzl Sorry for the extra conflicts with your PR but I'd like to merge this. I hope the conflicts won't be that difficult to resolve, and in the meanwhile this will make iterating over new tests much faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant