Skip to content
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

Fix usage of split_inclusive in event parser #31

Merged

Conversation

cwaldren-ld
Copy link
Contributor

@cwaldren-ld cwaldren-ld commented Mar 24, 2022

The test-decode-line-split-across-chunks unit test failed locally on M1 Mac with a panic due to unwrap(). It was not failing in CircleCi.

I then realized our CircleCi config was using the old rust image (circleci/rust), so I updated to cimg/rust, whereupon the test began failing as it did locally.

Due to rust-lang/rust#89825, split_inclusive now returns None when it used to return Some(empty slice) in our event parser.

The version of rust in circleci/rust is Rust 1.49.0, which is before the issue I linked above.

@cwaldren-ld cwaldren-ld changed the title Debugging a local failed unit test Fix usage of split_inclusive in event parser Mar 24, 2022
@cwaldren-ld cwaldren-ld marked this pull request as ready for review March 24, 2022 20:49
It was relying on old behavior where Some(empty slice) would be returned
if there were no matches. This was updated in more recent rust versions to return None,
but CircleCI was using a much older version of rust than I have locally.

Updated the usage and CircleCI config to use cimg/rust.
@cwaldren-ld cwaldren-ld force-pushed the cw/test-decode-line-split-across-chunks-failure-on-mac branch from cf385b5 to eda88f5 Compare March 24, 2022 20:58
@kinyoklion
Copy link
Member

Final question on this. Are we enforcing a min-version that has the new behavior versus the old?

@cwaldren-ld cwaldren-ld force-pushed the cw/test-decode-line-split-across-chunks-failure-on-mac branch from 2be2110 to c746827 Compare March 28, 2022 17:47
@cwaldren-ld cwaldren-ld merged commit 08a3496 into master Mar 30, 2022
@cwaldren-ld cwaldren-ld deleted the cw/test-decode-line-split-across-chunks-failure-on-mac branch March 30, 2022 17:49
keelerm84 added a commit that referenced this pull request Apr 11, 2022
keelerm84 added a commit that referenced this pull request Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants