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

fmt: use get_matches_from() instead of try_get_matches_from() in tests #6374

Merged

Conversation

cakebaker
Copy link
Contributor

While reviewing #6362 I noticed that we use try_get_matches_from() in the tests. We pass only valid values to this function, and so it's simpler to use get_matches_from().

Copy link

github-actions bot commented May 7, 2024

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@BenWiederhake
Copy link
Collaborator

Yes, the code is much easier to read.

However, when an unexpected failure occurs, this now kills the entire test suite due to the direct invocation of exit(2), instead of just showing an error for that single testcase.

Suggestion: Mostly your approach, but with try_get_matches_from(…).unwrap()
This way, the testcase fails, and we get a "normal" testcase failure (and all other tests continue as usual).

@cakebaker cakebaker force-pushed the fmt_use_get_matches_from_in_tests branch from e40bd02 to 3558dd3 Compare May 7, 2024 13:08
@cakebaker
Copy link
Contributor Author

Changes since last push:

  • used try_get_matches_from(…).unwrap() instead of get_matches_from (thanks to @BenWiederhake for the suggestion)

Copy link

github-actions bot commented May 7, 2024

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

Copy link
Collaborator

@BenWiederhake BenWiederhake left a comment

Choose a reason for hiding this comment

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

Simplifies the code, has no disadvantage that I can think of. Yay! :D

EDIT: CI failures are #6275 and #6333, as usual.

@BenWiederhake BenWiederhake merged commit b64183b into uutils:main May 7, 2024
67 of 68 checks passed
@cakebaker cakebaker deleted the fmt_use_get_matches_from_in_tests branch May 8, 2024 05:13
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