Vector: Run tests in `lib/*` on pull requests

Created on 13 May 2020  路  13Comments  路  Source: timberio/vector

Are tests run in the lib subdirectories? I just checked submitted a pull request that touched only files in lib/codec, and no tests were run. Looking at other pull requests, it doesn't appear tests are being run in lib at all.

Since changes to the sources in lib can fix or cause bugs in the rest of the code, we should at least be running standard tests when those change. Additionally, running tests in those directories would be a good idea.

ci tests must task

Most helpful comment

It may also make sense to remove the extra crates and just merge them into the main vector crate.

I think this would be a step backwards. IMO the more libs the better, particularly if they are stable, as it reduces the amount that needs to be compiled on each re-check cycle. I understand it won't help for the final link, which dominates the release build, but that happens much less frequently.

All 13 comments

This is a great idea.

We just need to pass a --all flag to the cargo test I believe.

--all alias for --workspace (deprecated), --workspace flag exists in script test-unit:
https://github.com/timberio/vector/blob/c39377cebd7809c25940d2578f49571735ac4aae/scripts/test-unit.sh#L11

It's already done.
Actually, --all is deprecated in favor of --workspace:

        --all                        Alias for --workspace (deprecated)
        --workspace                  Check all packages in the workspace

We need to add filters at GHA though.

It may also make sense to remove the extra crates and just merge them into the main vector crate.

It may also make sense to remove the extra crates and just merge them into the main vector crate.

I think this would be a step backwards. IMO the more libs the better, particularly if they are stable, as it reduces the amount that needs to be compiled on each re-check cycle. I understand it won't help for the final link, which dominates the release build, but that happens much less frequently.

But extra crates should reduce time required for building vector? Of course 3 current crates will not change it significantly, but currently wait more than 1min before tests will be actually launched is annoying.

I understand it won't help for the final link, which dominates the release build, but that happens much less frequently.

I think link happens much more often for regular debug workflows. Most of the time I've seen spent is in the link step. Especially for tests, where each tests test has to link against vector. Obviously, these small crates shouldn't make a difference.

but currently wait more than 1min before tests will be actually launched is annoying.

This is mainly due to the large size of rusoto and k8-openapi crate which are quite large. There may also be something with our C deps too.

It's already done.

I don't see any evidence in the GA logs that the codec and file-source tests are being run on PRs. If I'm reading the docker-compose.yml file right, the --all flag is only used on the test-integration and test-integration-aws targets. The former doesn't appear to be part of our GA process, and the latter doesn't run the tests in lib.

Am I missing something?

Sorry, I got confused, you're right! I corrected this at my PR https://github.com/timberio/vector/pull/2579 but it's still not merged.

Oh, wow, even in my PR it's not fixed. Yeah, we need a better way to handle tests. Good catch @bruceg.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

LucioFranco picture LucioFranco  路  3Comments

LucioFranco picture LucioFranco  路  3Comments

binarylogic picture binarylogic  路  3Comments

binarylogic picture binarylogic  路  3Comments

LucioFranco picture LucioFranco  路  3Comments