Move fault tolerant tests to separate module #12746
Conversation
.github/workflows/ci.yml
Outdated
| - { modules: testing/trino-tests, profile: test-fault-tolerant-hive-1 } | ||
| - { modules: testing/trino-tests, profile: test-fault-tolerant-hive-2 } | ||
| - { modules: testing/trino-tests, profile: test-fault-tolerant-delta } | ||
| - { modules: testing/trino-tests, profile: test-fault-tolerant-iceberg } |
There was a problem hiding this comment.
@nineinchnick @findepi
I am concerned this will probably run for each build on CI. Should we have separate, more isolated module for that?
Or separate modules for hive/delta/iceberg?
There was a problem hiding this comment.
What's the condition when these tests should be run, if not always?
There was a problem hiding this comment.
if trino-main/spi/etc change all should run.
Otherwise if specific connector changes (change in hive connector should trigger test--hive, ...)
There was a problem hiding this comment.
I am concerned this will probably run for each build on CI. Should we have separate, more isolated module for that?
like trino-recovery-tests instead of putting all this into trino-tests?
works for me too. i don't know whether it's warranted, i.e. whether this will cut down execution times.
i guess it matters / will matter more as more and more connector-related tests are moved into trino-tests; ultimately the module can become dependent on all connectors (but i don't think it will ever happen)
There was a problem hiding this comment.
however, it would be nice not to run all the tests when eg only Iceberg changes...
i'm fine with several additional modules, if this pays off with CI execution times.
There was a problem hiding this comment.
Splitting this up would also reduce the chance of flaky tests affecting more PRs (all run on master anyway). But if I'm not mistaken, iceberg also depends on hive. Anyway, it's worth extracting.
There was a problem hiding this comment.
Yeah I will extract. Single module for now trino-recovery-tests. Or should we go hard-core already and have separate for each connector?
There was a problem hiding this comment.
I will make one then - less pain - and should limit the burden already significantly.
losipiuk
force-pushed
the
lo/extract-fault-tolerant-tests
branch
2 times, most recently
from
6631ab7 to
2f90ee2
Compare
right link? |
...s/src/test/java/io/trino/faulttolerant/hive/TestHiveFaultTolerantExecutionConnectorTest.java
Outdated
Show resolved
Hide resolved
losipiuk
force-pushed
the
lo/extract-fault-tolerant-tests
branch
from
2f90ee2 to
266290f
Compare
|
AC |
|
fixups awesome. the only minor question is #12746 (comment) i.e. whether this is what we want to do ;) |
Thanks |
losipiuk
force-pushed
the
lo/extract-fault-tolerant-tests
branch
from
266290f to
e8a53f6
Compare
losipiuk
changed the title
losipiuk
force-pushed
the
lo/extract-fault-tolerant-tests
branch
2 times, most recently
from
4c41c22 to
d4579d4
Compare
Commit moves fault tolerant tests out of trino-hive, trino-iceberg, trino-delta to new trino-faulttolerant-test module. With fault tolerance tests in connector plugins those need to depend (in test scope) on trino-exchange-filesystem. This proved to be a problem and resulted in influencing compile classpath of plugins in a way that connectors were failing at runtime (some needed transitive dependecies were rescoped from compile to test). See: trinodb#12674
losipiuk
force-pushed
the
lo/extract-fault-tolerant-tests
branch
from
d4579d4 to
8e6c9c5
Compare
Move fault tolerant tests to separate module
Commit moves fault tolerant tests out of trino-hive, trino-iceberg,
trino-delta to new trino-faulttolerant-test module.
With fault tolerance tests in connector plugins those need to depend (in
test scope) on trino-exchange-filesystem. This proved to be a problem
and resulted in influencing compile classpath of plugins in a way that
connectors were failing at runtime (some needed transitive dependecies
were rescoped from compile to test).
See: #12674
Related issues, pull requests, and links
fixes #12674
Documentation
(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
(x) Release notes entries required with the following suggested text: