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

sharness/t0010: check that all the commands fail when passed a bad flag #4848

Merged
merged 2 commits into from
Mar 23, 2018

Conversation

chriscool
Copy link
Contributor

When any command is passed a flag it doesn't understand, like --badflag for example, it should just fail.

Let's check that in a sharness test.

See #4841 for more context.

@chriscool chriscool requested a review from Kubuxu as a code owner March 20, 2018 20:12
@ghost ghost assigned chriscool Mar 20, 2018
@ghost ghost added the status/in-progress In progress label Mar 20, 2018
echo 0 > fail
while read -r cmd
do
test_must_fail $cmd --badflag </dev/null >/dev/null ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with --help (#4841 (comment)), we shouldn't close stdin here.

Copy link
Contributor Author

@chriscool chriscool Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I will change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

while read -r cmd
do
test_must_fail $cmd --badflag </dev/null >/dev/null ||
{ echo $cmd exit with code 0 when passed --badflag; echo 1 > fail; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should definitely quote this. Also, we can probably just touch fail and check to see if it exists (simpler).

Copy link
Contributor Author

@chriscool chriscool Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About just touching fail, if we do that, we then have to make sure that we remove fail in case of failure, so that later tests that use the same mechanism don't fail just because of the leftover file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About quoting I tried to follow the same pattern as other tests. I will take a look at quoting everywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh. Let's just leave the failure mechanism as-is.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@chriscool
Copy link
Contributor Author

We are not closing stdin anymore.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@Stebalien Stebalien added RFM and removed status/in-progress In progress labels Mar 20, 2018
@chriscool
Copy link
Contributor Author

I don't understand again why codecov complains.

@Kubuxu do you know what happens?

@Kubuxu
Copy link
Member

Kubuxu commented Mar 23, 2018

@chriscool that is normal-ish.

@chriscool
Copy link
Contributor Author

@Kubuxu ok can you merge this one too?

@Kubuxu
Copy link
Member

Kubuxu commented Mar 23, 2018

@chriscool I can't, currently only Why can.

@whyrusleeping
Copy link
Member

Thanks @chriscool :) Welcome back!

@whyrusleeping whyrusleeping merged commit 550c6eb into master Mar 23, 2018
@whyrusleeping whyrusleeping deleted the check-failure-with-bad-flag branch March 23, 2018 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants