spacetelescope / pandokia

Regression tests framework.
Other
2 stars 4 forks source link

Fail on failure #73

Closed jhunkeler closed 2 years ago

jhunkeler commented 2 years ago

Once and for all.

This has been bothering me. Pandokia always returns "success" (0) when doing recursive testing, because there's apparently no way to get the process's status back up to the main program.

This checks the end of test summary stats for failure states and applies them accordingly. A new pandokia.common.cfg.status_failable dictionary controls which test status(es) can trigger an error event.

Original behavior:

$ pdk run -r tests/tests/pdkrun; echo $?

Result:

Summary of entire run:
pass=4 fail=2 error=0
0

New behavior:

$ pdk run --help
#...
-E / --stats-as-error
    exit non-zero based on test run statistics
    or set PDK_STATS_AS_ERROR to any value to enable

    Default is False
#...

# INVOKE - By environment
$ export PDK_STATS_AS_ERROR=1
$ pdk -r tests/tests/pdkrun; echo $?

# INVOKE - By long argument
$ pdk run --stats-as-error -r tests/tests/pdkrun; echo $?

# INVOKE - By short argument
$ pdk run -E -r tests/tests/pdkrun; echo $?

Results:

Summary of entire run:
pass=4 fail=2 error=0
1
vglaidler commented 2 years ago

I have no objection as long as the current default behavior of the exit status does not change, in case we have scripts that depend on it.

jhunkeler commented 2 years ago

Great! Yes, the default behavior is untouched.

There's a catch of course. The stats dictionary is only as accurate as the runner's error logic, and this change provides no benefit if an error occurs before reaching the runner's code. The upside is this will potentially flush out more places where error handling can be improved.

vglaidler commented 2 years ago

Great! Yes, the default behavior is untouched.

OK.

The upside is this will potentially flush out more places where error handling can be improved.

Well.. we are actively not interested in developing/improving Pandokia at this point. We're going to keep it going through the H2P migration project, and then move on to something else. Honestly I would have veto'd this PR on those grounds except that you sounded so gratified to have fixed something that has been annoying you for so long :)

At this point Pandokia is the venerable dust-covered machine in the corner that we do still rely on, but are content to let unused bits of it rust away.

cdsontag commented 2 years ago

At this point Pandokia is the venerable dust-covered machine in the corner that we do still rely on, but are content to let unused bits of it rust away.

👍 😢 ❤️ 👍

jhunkeler commented 2 years ago

Honestly I would have veto'd this PR on those grounds except that you sounded so gratified to have fixed something that has been annoying you for so long :)

Hahaha, thank you. I can finally have closure! :rofl:

At this point Pandokia is the venerable dust-covered machine in the corner that we do still rely on, but are content to let unused bits of it rust away.

Understood. I'll try to make due with what I have after this gets merged.