search

LLNL / ATS

ATS - Automated Testing System - is an open-source, Python-based tool for automating the running of tests of an application across a broad range of high performance computers.
BSD 3-Clause "New" or "Revised" License
7 stars 5 forks source link

ATS does not return error codes on failed tests #90

Closed white238 closed 1 year ago

white238 commented 2 years ago

This function says that it returns false on interrupted or failed tests but there isn't a return statement:

https://github.com/LLNL/ATS/blob/7ed78d69f087cae67ac78e4fd94b483b98c18a70/ats/management.py#L661-L683

This causes problems in CI because unless you parse the reports for failed tests, jobs do not mark themselves as failures.

dawson6 commented 2 years ago

Well, that is odd. Nobody has mentioned this before. But it should be fixable for sure.

dawson6 commented 2 years ago

I'll be looking at ATS again on Monday most likely as I have some other work I want to get off of the python3 branch and into main. I'll see if I canfix this then.

davidbloss commented 2 years ago

@white238 I am talking with @dawson6 about this now and I am going to take a more thorough look at this. At a glance it seems like something that has been there but not addressed for.... years.

MishaZakharchanka commented 1 year ago

@white238 I'm working on this issue now and so far I have main() returning false on: keyboard interrupts, ATS errors, and failed tests. Would you want/expect main() to return false on tests that have timed out and/or halted as well? Or in a more general sense, would you expect main to return false for anything but all tests running successfully?

white238 commented 1 year ago

Yes, I would expect it to return false for anything but the tests running successfully.

MishaZakharchanka commented 1 year ago

@white238 I believe that I have added the missing functionality and tests are passing on our side. Is there any specific way that you would like to test this? I can make a public install for you or you can try with either the the branch linked to this issue or once the changes are pushed to the main branch.

white238 commented 1 year ago

Thanks for looking into this. I will try to try this out soon but I am fine with you guys closing this and leaving it up to me to test.