remove ndc-citus #105
remove ndc-citus #105
Conversation
| //! If you have changed it intentionally, run `just generate-chinook-configuration`. | ||
|
|
||
| #[cfg(test)] | ||
| mod configuration_tests { |
There was a problem hiding this comment.
It would seem to me that there should only be a single (rust) version of this test, but we should be able to point it at different database instances and snapshots, since that's the only thing that differs, right?
(and the same reasoning goes for the other tests.)
There was a problem hiding this comment.
Agreed, but I think we can do this later, as part of a concerted effort to remove duplication in tests.
There was a problem hiding this comment.
I'm fine with that as well. I just wanted to highlight it :-)
There was a problem hiding this comment.
I'll do it now. It'll make it easier to copy for cockroach and postgres later. thanks.
There was a problem hiding this comment.
I'll unify just the common tests for now actually.
| @@ -12,14 +12,24 @@ aurora = [] | |||
| # We only run the Yugabyte tests if this feature is enabled. | |||
| # Note that the Yugabyte Docker image seems not to work on aarch64. | |||
| yugabyte = [] | |||
| # We only run the Cockroach tests if this feature is enabled. | |||
| cockroach = [] | |||
| # We only run the Citus tests if this feature is enabled. | |||
There was a problem hiding this comment.
Why are we adding these as features? They only exist for yugabyte / aurora because those can't always run locally, whilst citus and cockroach can.
There was a problem hiding this comment.
Oh I see what you're doing here. Seems fine.
.github/workflows/cargo-test.yaml
Outdated
| @@ -190,3 +189,40 @@ jobs: | |||
| cargo nextest run --no-fail-fast --release -p other-db-tests --features yugabyte | |||
| env: | |||
| RUST_LOG: INFO | |||
|
|
|||
| test-ndc-postgres-with-citus: | |||
There was a problem hiding this comment.
Consider matrix-ifying this please. Doesn't have to be now.
There was a problem hiding this comment.
Use a matrix strategy (just like the test-connector job above) instead of copying and pasting.
It's a fancy loop.
There was a problem hiding this comment.
oh I understand. Thanks.
What
Since ndc-citus is essentially exactly like ndc-postgres, and we don't see that changing in the near future, we'd like to remove the ndc-citus binary but keep testing against citus.
How
other-db-testsfor citusndc-citus