LLNL/ATS

ATS does not return error codes on failed tests

Closed this issue · 7 comments

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

ATS/ats/management.py

Lines 661 to 683 in 7ed78d6

def main(self, clas = '', adder=None, examiner=None):
"""
This is the main driver code.
Returns true if all interactive tests found passed, false if interrupted or
an error occurs.
``clas`` is a string containing command-line options, such as::
--debug --level 18
If ``clas`` is blank, sys.argv[1:] is used as the arguments.
See ``configuration.init``.
Routines ``adder`` and ``examiner``, if given, are called in ``configuration``
to allow user a chance to add options and examine results of option parsing.
"""
self.init(clas, adder, examiner)
self.firstBanner()
self.core()
self.postprocess()
self.finalReport()
self.saveResults()
self.finalBanner()

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

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

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.

@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.

@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?

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

@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.

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.