fix(unit tests): Panic caused by edge case during unit test build #23628
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
I was working on the unit test included in the config documented below which is invalid because
filter_msg
is listed in bothno_outputs_from
andoutputs.extract_from
.The issue arises when you try to run the invalid test, instead of handling the issue cleanly, Vector panics instead:
This seems to happen because Vector doesn't include a check that
output.extract_from
entries are not also listed inno_outputs_from
which allows it to pass the build phase and fail when the invalid test is unwrapped, causing the panic.This PR adds a check which resolves the issue and lets Vector exit cleanly with a defined error that follows the format used by another error in the same function.
PS: This is my first time raising a PR on an open source project so any feedback on how I raised / worded this PR would be greatly appreciated! (I hope its ok I made a PR without making an issue first? The contributing page said "For large PRs or for breaking changes, ensure your change has an issue!" I couldn't find another issue raised about this and figured this wasn't a large or breaking change)
Vector configuration
How did you test this PR?
I trialed similar edge cases I could think of to ensure they also pass with this new check as well as running the suggested tests:
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changelog
label to this PR.References
Notes
@vectordotdev/vector
to reach out to us regarding this PR.pre-push
hook, please see this template.cargo fmt --all
cargo clippy --workspace --all-targets -- -D warnings
cargo nextest run --workspace
(alternatively, you can runcargo test --all
)git merge origin master
andgit push
.Cargo.lock
), pleaserun
cargo vdev build licenses
to regenerate the license inventory and commit the changes (if any). More details here.