Skip to content

Conversation

@catamorphism
Copy link
Contributor

Add tests for the -h and --help output for the
parse, print, and validate commands.

@catamorphism catamorphism requested a review from a team as a code owner September 30, 2025 20:18
@catamorphism catamorphism requested review from fitzgen and removed request for a team September 30, 2025 20:18
@alexcrichton
Copy link
Member

Poking through clap sources, setting the COLUMNS=... env var may work for the failures here, but that'll only work if the terminal-size crate otherwise can't find the dimensions of stdout/stderr/stdin/etc. I think though we could arrange for everything to be a pipe (or /dev/null) to subprocesses in tests to force that to fail, and then we'd always set .env("COLUMNS", "80") or something like that and it might force the same-width everywhere?

@catamorphism
Copy link
Contributor Author

I think it's already a pipe? https://github.com/bytecodealliance/wasm-tools/blob/main/tests/cli.rs#L155

I added .env("COLUMNS", "80") and it works for me locally, although of course it worked for me locally in the first place.

@alexcrichton
Copy link
Member

Ah the old nemesis of Windows... Looks like the issue is:

       <Usage: wasm-tools parse [OPTIONS] [INPUT]
       >Usage: wasm-tools.exe parse [OPTIONS] [INPUT]

Maybe a hammer like this could work where the tests just replace .exe with nothing and see how that works?

@catamorphism
Copy link
Contributor Author

Green now :)

let mut output = String::from_utf8_lossy(output)
.replace(tempdir, "%tmpdir")
.replace("\\", "/")
.replace("wasm-tools.exe", "wasm-tools")
Copy link
Member

Choose a reason for hiding this comment

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

Much better than my idea!

@alexcrichton alexcrichton added this pull request to the merge queue Sep 30, 2025
Merged via the queue into bytecodealliance:main with commit 9b39693 Sep 30, 2025
34 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.

2 participants