Skip to content

Conversation

@karenetheridge
Copy link
Member

While some consider this not best practice, blessed objects that overload to a false result are legitimate and should not be prevented.

While some consider this not best practice, blessed objects that overload to a
false result are legitimate and should not be prevented.
@haarg
Copy link
Member

haarg commented Jan 22, 2025

Is this something you ran into in real code?

This behavior matches what Test::Fatal does. Quoting its docs:

Achtung! If the block results in a false exception, such as 0 or the empty string, Test::Fatal itself will die. Since either of these cases indicates a serious problem with the system under testing, this behavior is considered a feature. If you must test for these conditions, you should use Try::Tiny's try/catch mechanism. (Try::Tiny is the underlying exception handling system of Test::Fatal.)

If this was merged, the message inside the unless condition would probably need to be updated, as it wouldn't be complaining about every falsy exception.

@karenetheridge
Copy link
Member Author

Yes, I am working with an object that has a boolean overload, where false indicates a negative outcome and true indicates a successful one. A method that returns one of these objects can also throw an exception, which naturally I would like to wrap tests around.

An exception consisting of 0 might be considered an error worth warning on if it is a plain scalar, but objects with overloads are different.

@haarg
Copy link
Member

haarg commented Jan 22, 2025

It's a method that can return one of the objects or throw the same type of object? It sounds like a very weird interface.

$@ having a value of 0 could pretty much only happen if someone assigned it explicitly while the stack was being unwinded (on vulnerable perl versions). But that doesn't really have any bearing on this change.

This probably needs a test.

The docs include a section DIFFERENCES FROM TEST::FATAL. This difference should probably be noted there.

@exodist
Copy link
Member

exodist commented Jan 22, 2025

I agree with @haarg needs some doc updates and a test, then I am willing to merge this. Though I also have to be sure nothing downstream breaks as it is possible other test tools depend on (and test for) this behavior, though that is pretty unlikely.

@karenetheridge
Copy link
Member Author

It's a method that can return one of the objects or throw the same type of object? It sounds like a very weird interface.

No, it's a constructor method that returns a thing, or throws something else (which has a success/fail? boolean overload, as it is returned as the normal result in other places) if there was an error during construction.

My opinion would be that Test::Fatal should change as well -- I've always thought it odd that it is opinionated about falsey exceptions -- but given its age the ramifications of that change might be greater. Test2's exception plugin is much newer so it is easier to change to do the "more right" thing.

@karenetheridge karenetheridge marked this pull request as draft January 23, 2025 01:06
@karenetheridge
Copy link
Member Author

16:08 < ether> there actually is no way of testing the normal case (an exception that is not blessed), because it doesn't seem to be possible to throw an exception that is undef, '' or 0 -- the " at file, 
               line" string is always appended when there is no newline in the exception.
16:22 < Exodist> not on current perl. On older perls you could set $@ = 0 at the right time to make it happen.
16:26 < ether> how old?
16:26 < dipsy> how old is your audience
16:36 < Exodist> that I do not know. I just remember a time when $@ could not be trusted to be true after an eval that did throw an exception, and that recent perls fixed it.
16:36 < Exodist> haarg may know?
16:37 < ether> if it's early enough we might be able to stop working around that bug
16:39 < Exodist> This has 1, 0 on 5.8.8, so that far back it is broken, need to check all the in-betweens https://www.irccloud.com/pastebin/I4IGrZMp/
16:40 < ether> I suppose I could also check $] and do the workaround for affected versions only
16:40  * ether greps perldeltas...
16:41 < Exodist> looks like it was fixed in 5.14, according to my perlbrew exec
16:42 < Exodist> perlbrew exec results https://www.irccloud.com/pastebin/nx1FpxD0/out.txt
16:47 < ether> ah, it's specific to destructors
16:48 < ether> ok what I want to do is update Test::Fatal to only warn on falsy values for older perls where it's possible for a falsy non-blessed exception to happen
16:48 < ether> and then port that change to Test2::Tools::Exception

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants