CLEAN can be borked too

[kamil-tekiela]

Exceptions sometimes happen in CLEAN sections. There's no reason why we should not catch them and fix these tests.

[nikic]

CLEAN sections can run either if the test succeeded or failed. If it failed, maybe some files haven't been created yet. There's two ways we could handle this:

• Make it the convention that all unlink(), rmdir() etc in CLEAN should go under @, which makes them robust against not-existing files. The disadvantage is that we won't notice if they're unlinking/rmdiring the wrong thing (e.g. there were a bunch of CLEAN sections deleting files that have no relation to the test...)
• Only perform the BORK check if the test passed. That means that CLEAN should run without warnings if the test succeeded, but it may throw warnings if it failed, and those will not be reported.

[kamil-tekiela]

@nikic While I can accept that some tests can use @ on unlink to clean up temporary files, I was more interested in catching exceptions that are impossible to be silenced with @. So I think the better option would be to run it on SUCCESS only, but that is a pity since other sections need clean up too. For example, the test I was fixing was doing a lot of operations in SKIPIF. How to clean that up properly without borking CLEAN?

[nikic]

@kamil-tekiela I'm not suggesting that CLEAN isn't run (the whole point of the section is that it runs even on failure), just that we don't report BORK if the test is already failing. I pushed a commit to implement what I had in mind.

[kamil-tekiela]

I see I wasn't very clear with my previous message (even I can't comprehend what I wrote).
What I meant is: If CLEAN borks on successful tests, then report it like you suggested. When CLEAN borks on a failed test, then don't report it. I think this is what you are suggesting. My point is that when a test is not even executed because of SKIPIF and the CLEAN section borks, we should also report it.

[nikic]

@kamil-tekiela At least right now, CLEAN is never executed for skipped tests. It only gets run if the test runs. SKIPIFs are responsible for their own cleanup.