tighter use of try, catch

[cbm755]

Something I dislike about Octave's __run_test_suite__ is that failures in its own code don't necessarily raise errors.

Here's a bug I just found where there is an error in the code but its not raised b/c (I guess) the unwind protect stuff: https://savannah.gnu.org/bugs/index.php?55838

Update: I guess I meant https://savannah.gnu.org/bugs/?55841?

See also https://github.com/apjanke/octave-testify/blob/master/inst/__run_test_suite__.m#L185

I think try/catch and unwind_protect should be used sparingly: whenever I see large blocks of code wrapped in those I think its going to hide bugs...

[apjanke]

Sorry for the slow response on this. I didn’t have “Watch” turned on for this repo, so I wasn’t getting notifications for bug report submissions.

Sounds good. I'm going to call this a bug instead of a feature request, because it’s not adding new functionality, just fixing how the existing code works.

[apjanke]

Did some in https://github.com/apjanke/octave-testify/commit/130246f6096ac7ee98deb9bb90f8694f59e00a70 and https://github.com/apjanke/octave-testify/commit/7a2b4f3c00f161e2180624033a1eabc25919b4fb.

[apjanke]

Did one more in https://github.com/apjanke/octave-testify/commit/4be1bad8a4bbbecd2b2e769f909d002aae010b06. That's most of Testify's high-level try/catches gone. Now internal errors in the test code will bubble up to the top and abort the test, and can be debugged with dbstop if error.

[apjanke]

Do you actually care about unwind_protect? AFAIK, it doesn't interfere with dbstop and doesn't keep errors from propagating up to the top.

[cbm755]

I'm not sure I understand enough to answer that, as I'm unsure of the precise differences b/w try-catch and unwind_protect. To me, they both encourage some kind of magical thinking instead of proper error checking: but that may not be accurate at all!

[apjanke]

unwind_protect just ensures that the code in the cleanup block always executes, regardless of whether the protected block was exited via normal falling-off-the-end, an early return, or a raised error(). It’s basically a way of avoiding copy-and-pasting cleanup code into multiple spots inside the main block. There is no actual error handling done in unwind_protect; its only interaction with error handling is to delay propagation of the return value or exception until the cleanup code executes.

It’s very similar to onCleanup or handle object destructor behavior, except tied to a syntactic code scope instead of an object lifetime.

If your main concern is that the internal test code doesn’t raise errors, then I think unwind_protect is irrelevant.

[cbm755]

Ok, thanks for the explanation.

[apjanke]

This turned out to be a good idea; debugging the code was easier this way.

I think I've got all the offenders. The remaining try/catches are either necessary or don't interfere with debugging, and are scoped pretty narrowly. Closing as fixed.