test harness doesn't fail on some errors

Reporter: fundies  |  Status: open  |  Last Modified: April 09, 2020, 11:52:30 AM

The terst harness doesn't fail in a few scenarios where it should. Such as when it fails to post a comment or it fails to post the code coverage it will not mark the job as failed
RobertBColton  
Well the image differences detected comments are not supposed to be failures, just suggestions remember. For example, expanding a draw test would trigger that but doesn't necessarily mean anything regressed or that the test failed.

I consider these to be special case scenarios, where these types of things (e.g, code coverage not posting) usually should never occur, and that they really shouldn't fail the test harness. For example, if code coverage server is having issues causing EnigmaBot not to be able to comment, that's not really our fault and the test harness should not be failed. The build log should contain all of the information necessary to decide whether to merge a pull request, such as the change in code coverage or where image differences were detected, which it already does.

I don't see a problem here at all really.

fundies  

if there is a regression and the bot fails to upload it we would never know until the bot continues working again. this is pretty big issue
RobertBColton  

I was thinking we'd know because the log would say differences detected, but I see now. Yeah, there's no way we'd know, and this could have already happened to us as well and we could have merged a pull request thinking it's passing when it really isn't.

In that case then, only in these specific cases do I support changing it to fail.

Please sign in to post comments, or you can view this issue on GitHub.