Multi-line test commands can spuriously pass

[itamarst]

Description

Let's say CIBW_TEST_COMMAND="false; true". On Unix this will be considered a success, even though one of the commands failed.

Here's a real example that tripped me up:

[tool.cibuildwheel]
test-command = """\
pip install -r {project}/requirements-dev.txt
pytest {project}/ -k 'not memory_usage'  # <-- this failed
pytest {project}/ -k 'memory_usage'      # <-- this passed
"""

Now, you could say that this is my responsibility and I should just add a set -e. But set -e I assume won't work on Windows, where the same commands will be run. So making sure failing commands cause tests to not spuriously pass is probably something cibuildwheel should do.

(I can do separate tests for Linux and Windows, but that's a workaround which leads to unnecessary duplication of scripts. This feels like a footgun that has probably hit other projects too. There's a reason most CI systems have set -e as the default.)

Build log

No response

CI config

No response

[henryiii]

You can use && or a list of strings and cibuildwheel will chain them with && for you. Works on all OSs.

[tool.cibuildwheel]
test-command = [
  "pip install -r {project}/requirements-dev.txt",
  "pytest {project}/ -k 'not memory_usage'",
  "pytest {project}/ -k 'memory_usage'",
]

[itamarst]

Thanks, good to know! The default still feels like a footgun though.

[joerick]

I agree, it is a shame, and it is a footgun. Unfortunately I don't think there is a way to get set -e behaviour on the default shell on Windows. I think we discussed it in the past, my feeling was that it was an even worse footgun if we only have the bad behaviour on Windows, since people are less likely to notice and fix the issue on one platform than across the whole build.

Options would be:

• I would love to switch to a better shell on Windows. I wonder if pwsh could work here? Is it always available? Is it compatible-ish with macos/linux? It would be a breaking change, so would have to be a v3.0 kinda thing I think.
• Detect shell commands that have multiple statements and show a warning if set -e or set -o errexit doesn't also appear? We do have bashlex already as a dependency. But warnings don't exactly 'solve' the problem, you still have to spot them in the logs.

[itamarst]

Seems Powershell also has bad defaults 😢 With latest versions apparently this is how to fix them: PowerShell/PowerShell#3415 (comment)