-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(ci): run integration tests in parallel #22205
Conversation
ca24143
to
4d6d2f2
Compare
Datadog ReportBranch report: ✅ 0 Failed, 7 Passed, 0 Skipped, 25.43s Total Time |
4d6d2f2
to
1a05c38
Compare
3c619a2
to
5d660fe
Compare
d57968a
to
5a5ef92
Compare
src/sinks/datadog/logs/tests.rs
Outdated
@@ -569,4 +569,5 @@ async fn override_global_options() { | |||
assert!(keys | |||
.iter() | |||
.all(|value| value.to_str().unwrap() == "local-key")); | |||
// test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: remove
Added to demonstrate ITs run: https://github.com/vectordotdev/vector/actions/runs/12794660202
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And after adding retries: https://github.com/vectordotdev/vector/actions/runs/12795081335
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comment, non-blocking spelling nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I'm excited to see this change to cut the long pole of the merge queue. It'll be nice to get feedback on each integration test separately too rather than it aborting on the first one.
.github/workflows/integration.yml
Outdated
|
||
- run: docker image prune -af --filter=label!=vector-test-runner=true ; docker container prune -f | ||
- run: docker image prune -af ; docker container prune -f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may not be necessary if these are run in parallel rather than sequentially. We had to do this cleaning when running sequentially since disk space would slowly be eaten up by successive runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I will keep an eye out for disk space errors. But I also think it can be removed now.
Summary
The IT suite is the longest running check in the merge queue. This PR splits the IT suite in several jobs that run in parallel.
Note: The jobs still build Vector individually. I will follow-up so that we build Vector once and then we re-use the binary across the suite.
Change Type
Is this a breaking change?
How did you test this PR?
Does this PR include user facing changes?
Checklist
make check-all
is a good command to run locally. This check isdefined here. Some of these
checks might not be relevant to your PR. For Rust changes, at the very least you should run:
cargo fmt --all
cargo clippy --workspace --all-targets -- -D warnings
cargo nextest run --workspace
(alternatively, you can runcargo test --all
)Cargo.lock
), pleaserun
dd-rust-license-tool write
to regenerate the license inventory and commit the changes (if any). More details here.References