Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@test_throws: don't throw an error if the same field is undefined in both the observed exception and the expected exception #54084

Closed
wants to merge 2 commits into from

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Apr 14, 2024

Fixes #54082
Alternative to #54083


This is another possible way to fix #54082.

…n both the observed exception and the expected exception
@DilumAluthge DilumAluthge marked this pull request as ready for review April 14, 2024 21:16
@martinholters
Copy link
Member

Could use a test. And that would probably reveal that this does not fix #54082 IIUC, as in that case, the thrown exception has .scope set while it is #undef in the expected one. So for the test case of #54082, what used to be a passing test would be "fixed" to be a failing test instead of thrown exception. (Similarly for #54032, btw.) Or am I missing something?

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 17, 2024
@nickrobinson251
Copy link
Contributor

i don't think this fixes #54082

Please can tests be added?

@IanButterworth IanButterworth added needs tests Unit tests are required for this change and removed merge me PR is reviewed. Merge when all tests are passing labels Oct 17, 2024
@DilumAluthge
Copy link
Member Author

And that would probably reveal that this does not fix #54082 IIUC, as in that case, the thrown exception has .scope set while it is #undef in the expected one. So for the test case of #54082, what used to be a passing test would be "fixed" to be a failing test instead of thrown exception.

Yeah, Martin and Nick are correct, this doesn't fix #54082 for the reason described.

@DilumAluthge DilumAluthge deleted the dpa/test_throws-undefined-fields branch October 17, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user needs tests Unit tests are required for this change testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change in behavior of UndefVarError(::Symbol) constructor breaks @test_throws in 1.11.0-beta1
6 participants