Fix MethodError while showing MethodError from invoke with UnionAll#47566
Fix MethodError while showing MethodError from invoke with UnionAll#47566Seelengrab wants to merge 2 commits into
MethodError while showing MethodError from invoke with UnionAll#47566Conversation
|
Please use descriptive titles and commit messages. |
MethodError while showing MethodError from invoke with UnionAll
682b76c to
69666c0
Compare
| @testset "issue #47559" begin | ||
| err = try; invoke(Returns, Tuple{Any,Val{N}} where N, 1, Val(1)) catch ex; ex; end | ||
| @test startswith(sprint(Base.showerror, err), "MethodError: no method matching Returns(::Any, ::Val{N})") | ||
| end |
There was a problem hiding this comment.
Looks like a good start. Unfortunately, I have already worked with Aaron to write a more complete version of this PR (there are some additional subtleties to cleanup too around the use of the parameters). As a minor point of reference, for very new issues, you may want to explicitly confirm on Slack that it is not already being worked on, or at least assign yourself to it, to avoid repeating work.
This test may be helpful though, since we had not written that yet (@apaz-cli).
There was a problem hiding this comment.
Gotcha - the errorshow tests ran through locally, so I thought it should work (the test faillures seem to be unrelated - ReadOnlyMemoryError in SparseArrays?). I only looked at the issue in the first place since it had to do with showerror of MethodError, so I thought it was related to the printing changes. Feel free to close this if you've already worked out a better solution than my hacky unwrap_unionall :)
Assigning myself is not really an option without write access, but I'll ping on slack next time! 😅 The test more or less follows the pattern at the start of errorshow.jl, except with the sprint inside of the @test to capture that failure correctly.
The old code had a comment about this being either a
Tupleor a type, and chose to check for the type. Unwrapping theUnionAlland checking for being a tuple seemed to be the easiest way to fix this, so that's what I did.Also adds the same comment from
showerrortoshow_method_candidates, since that assumes the same thing.Fixes #47559