Lit runner passing empty stdin to tool makes incorrect tests pass

I recently (720dd81) fixed a test that was missing a %s argument in the RUN: directive.

The test passed on lit because the lit runner provides an empty string on stdin, but didn’t actually check the input as intended. This was only detected because in Google’s test runner, the test hung with mlir-opt waiting for input.

I’ve actually made the same mistake before and I would like to stuff that trap with D135067. Is it OK to change the semantics of the lit runner to provide no instead of an empty stdin? No new tests start failing in LLVM, but maybe someone is aware of downstream projects that would rely on this behaviour?


1 Like

No new tests start failing in LLVM

I was wrong about this, I updated D135067 to fix a handful of clang tests that started failing. As far as I can tell, none of them were using an empty input accidentally.

I would expect a test environment to close stdin instead? That is: we’re running tools absent from a shell there is no reason to leave it open. If the test passes without a stdin, why should we prevent it?

Agree, it definitely should not propagate stdin from the caller of llvm-lit (which is what “None” does).

I can see an argument either for empty (like now), an indefinitely-hanging read (e.g. unclosed pipe with no data in it), or, if you wanted to get REALLY fancy, could even make a pipe which is verified to not be read from by any of the tests. But whatever the choice, it should be independent of the caller’s environment.