Skip to content

Add test coverage for text mode error#231

Merged
hukkin merged 2 commits intohukkin:masterfrom
hauntsaninja:coverage
Oct 8, 2024
Merged

Add test coverage for text mode error#231
hukkin merged 2 commits intohukkin:masterfrom
hauntsaninja:coverage

Conversation

@hauntsaninja
Copy link
Copy Markdown
Collaborator

Error message was added in #175

@hukkin
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@hukkin
Copy link
Copy Markdown
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