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

Add test coverage for text mode error #231

Merged
merged 2 commits into from
Oct 8, 2024
Merged

Conversation

hauntsaninja
Copy link
Collaborator

Error message was added in #175

@hukkin
Copy link
Owner

hukkin commented Oct 2, 2024

Thanks!

It seems you did add a test in #175 (tests/test_misc.py/test_incorrect_load). Should we add the error message assertion in that one instead?

That test should also probably live in the test_error.py file, but since it deviates us from cpython/tomllib I'm not sure it's worth moving it.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. Missed that because I was just looking at the diff in your PR #229 :-)

I added the message in the location of the existing test. If we want to move it that's fine too (I can make the corresponding change in CPython)

@hukkin
Copy link
Owner

hukkin commented Oct 4, 2024

If you have the energy to move it in CPython, let's do it here too in this PR! If not then this LGTM.

@hukkin hukkin merged commit f57fb66 into hukkin:master Oct 8, 2024
26 checks passed
@hukkin
Copy link
Owner

hukkin commented Oct 8, 2024

I'll actually just merge this now. We can consider moving the test separately.

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.

2 participants