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

timeout: avoid panicking for empty string #3206

Merged
merged 1 commit into from Mar 6, 2022

Conversation

353fc443
Copy link
Contributor

@353fc443 353fc443 commented Mar 2, 2022

Related to #3040

@sylvestre
Copy link
Sponsor Contributor

Could you please add a test to make sure we don't regress in the future? thanks

@353fc443
Copy link
Contributor Author

353fc443 commented Mar 5, 2022

Hi, I'm unable to invoke the error from #3040 using new_ucmd macro

@sylvestre
Copy link
Sponsor Contributor

#[test]
fn test_command_fail() {
    new_ucmd!()
        .args(&["\"\"", "\"\""])
        .fails()
        .stderr_contains("thread 'main' panicked at");
}

works for me in tests/by-util/test_timeout.rs
(just replace the error message by the new one)

@353fc443
Copy link
Contributor Author

353fc443 commented Mar 6, 2022

Unfortunately the error in #3040 is not produced using the above test also @sylvestre .

Tested on Linux and MacOS

Diff < left / right > :
<"timeout: invalid time interval '\"\"': invalid float literal"
>"empty string"

', tests/common/util.rs:326:9

@sylvestre
Copy link
Sponsor Contributor

ok, i will give it a try :)
thanks

@sylvestre sylvestre merged commit 9113594 into uutils:main Mar 6, 2022
@sylvestre
Copy link
Sponsor Contributor

I created this:
#3218
(the " were not necessary)

@353fc443 353fc443 deleted the timeout-empty-string branch March 6, 2022 23:34
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.

None yet

2 participants