I am looking at the fixes you apply to sanitizer tests and they worry me.
(e.g. https://reviews.llvm.org/D31498)
The fixes are mostly mechanical and thus every single change looks safe,
but given the amount of changes there is large risk to cripple some of the tests
in a way that they will stop detecting failures.
When I write a test for new functionality, I always verify that the test fails w/o the implementation
of that functionality (i.e. that the test catches the failure). I hope most of the tests are developed this way too. But when you modify the tests, I bet you only verify that the passing test is still passing. Am I right? If yes, this is not the way to modify tests, unfortunately.
So, my question is: did you consider modifying the environment to closer match what already works well on posix, instead of massively changing tests?
I am looking at the fixes you apply to sanitizer tests and they worry me.
(e.g. https://reviews.llvm.org/D31498)
The fixes are mostly mechanical and thus every single change looks safe,
but given the amount of changes there is large risk to cripple some of the
tests
in a way that they will stop detecting failures.
When I write a test for new functionality, I always verify that the test
fails w/o the implementation
of that functionality (i.e. that the test catches the failure). I hope most
of the tests are developed this way too. But when you modify the tests, I
bet you only verify that the passing test is still passing. Am I right? If
yes, this is not the way to modify tests, unfortunately.
It is probably not as comprehensive and testing with the functionality
disabled, but I do tent to try to break the test locally. For example,
when changing from to count I will check that the test does catch
having the wrong count.
So, my question is: did you consider modifying the environment to closer
match what already works well on posix, instead of massively changing tests?
Yes, unfortunately there is no good way to do that as far as I can
tell. Different posix shells on windows have different oddities and
they break from time to time (which is probably why we have an
internal shell in the first place).
Currently the only big feature that I am changing tests for is
avoiding a subshell. The rest I seem to be able to implement in the
internal shell. I have just started sending the shell patches up for
review.
The difference to the posix tests are fairly small. I have attached a
diff showing all that is left from current trunk.
We could make lit tests more portable by unconditionally setting execute_external to False in lit.common.cfg. This would use the lit shell on all platforms, and make it impossible to accidentally write tests that don’t run on Windows from Posix.
We could make lit tests more portable by unconditionally
setting execute_external to False in lit.common.cfg. This would use the lit
shell on all platforms, and make it impossible to accidentally write tests
that don't run on Windows from Posix.
We have a long term goal to do this anyway: https://bugs.llvm.org/
/show_bug.cgi?id=5241 Want to be the first LLVM project to do it?