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

Implement change proposed in "All MRs that add unit tests conflict with each other" #602

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

quag
Copy link
Contributor

@quag quag commented Oct 1, 2023

Fixes #420 using the proposed approach of defining ok(), done_testing(), and shifting the TAP plan from being written at the start to being written at the end. I also introduced an ok_skip() function, but I'm happy to change all uses to ok "# SKIP ..." if you prefer that.

This pull request also has two trivial fixes to the tests:

  1. A typo in a test case ("prefxing" -> "prefixing"
  2. Switch the last /bin/bash to just bash (there are five references to bash, the other four--including #!/usr/bin/env bash-- all use the bash on the $PATH)

Let me know if you'd rather these two changes were in separate pull requests.

Copy link
Collaborator

@smcv smcv left a comment

Choose a reason for hiding this comment

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

This will need a rebase onto main with a small adjustment now that I've merged #598, but otherwise looks good. Thanks!

@smcv
Copy link
Collaborator

smcv commented Oct 1, 2023

Rebasing after I merged #599 will also require adding one extra ok_skip, I think.

Fixes containers#420

Signed-off-by: Jonathan Wright <quaggy@gmail.com>
Signed-off-by: Jonathan Wright <quaggy@gmail.com>
The other three references to bash already use "bash" instead of
"/bin/bash". Similarly, "#!/bin/bash" has already been replaced with
"#!/usr/bin/env bash".

Signed-off-by: Jonathan Wright <quaggy@gmail.com>
@quag
Copy link
Contributor Author

quag commented Oct 2, 2023

Rebases done. Let me know if more are needed.

@smcv smcv merged commit 9cdc26e into containers:main Oct 2, 2023
5 checks passed
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.

All MRs that add unit tests conflict with each other
2 participants