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(tests): restrict 'cat' tests to unix environments. Fixes #776 #777

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ritvikos
Copy link

Tests using the cat command are specific to Unix-like environments and will fail on unsupported platforms.
Restrict these tests to unix environments only.

Tests using the 'cat' command are specific
to unix-like environments and
fail on unsupported platforms.
@sharkdp
Copy link
Owner

sharkdp commented Dec 1, 2024

Thank you for reporting this and for opening a PR. It would be great if we could test this behavior on Windows somehow. Maybe we can find a Windows replacement for cat? Doesn't need to be a cat-equivalent. Just something that we can pass input to. And check if it received the input correctly.

@ritvikos
Copy link
Author

ritvikos commented Dec 2, 2024

Great idea!
Found that type is the replacement of cat in Windows.
Now the tests work fine 😄

.arg("--runs=1")
.arg("--input=example_input_file.txt")
.arg("--show-output")
.arg("type example_input_file.txt")
Copy link
Owner

Choose a reason for hiding this comment

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

This is not testing the right thing, I believe? The cat test above on Linux makes sure that we can pipe input from example_input_file.txt into the benchmarked command. But this looks to me like you ignore that and simply output the contents if example_input_file.txt using type.

@ritvikos
Copy link
Author

type cannot handle stdin like cat, just prints the content of the file.

Whoops! I missed that detail 😅
Nice catch!

Although, found 2 alternatives that can handle pipe stdin on Windows,
let me know if you think it looks acceptable or you want me to look into other alternatives 😄

  1. findstr x*: StackOverflow

someprog | findstr x* or any other single character followed by
asterisk copies all lines from the pipe stdin to stdout.

~\Desktop\oss\hyperfine\tests> hyperfine --runs=1 --input=example_input_file.txt --show-output "findstr x*"
Benchmark 1: findstr x*
This text is part of a file
  Time (abs ≡):         21.8 ms               [User: 0.0 ms, System: 4.7 ms]

  1. more command: Displays one screen of output at a time.
    Microsoft - More and Unixtutorial.org
~\Desktop\oss\hyperfine\tests> hyperfine --runs=1 --input=example_input_file.txt --show-output "more"
Benchmark 1: more
This text is part of a file
  Time (abs ≡):         16.2 ms               [User: 0.0 ms, System: 5.9 ms]

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