Skip to content

Conversation

dimtion
Copy link

@dimtion dimtion commented Sep 21, 2025

Allow user to add an optional message to failed assertions in mini test. This allow providing more context to specific failures when they arise. This is a similar feature that other assertion libraries support.

Usage:

MiniTest.expect.equality(x, y, "Invalid number of items")
MiniTest.expect.no_equality(x, y, "Unexpected line counts")
MiniTest.expect.reference_screenshot(screen, path, nil, "Buffer not matching")

I did not implement it for the errorand no_error assertions given that I'm not sure what would be the best API grammar for those two.

@echasnovski
Copy link
Member

Thanks for the PR!

This looks reasonable. Prepending the message before context also looks like a good approach (instead of completely overriding it).

I'd ask to make it more forward compatible and introduce table opts as a third argument. It can then have opts.msg as a possible field.

It would be good to also add it to all expectations, but it will be a breaking change for expect.error and expect.no_error. Maybe it will be worth it to replace ... with a single opts and allow opts.args to pass arguments... I'll have to think about it.

Allow user to add an optional message to failed assertions in mini test.
This allow providing more context to specific failures when they arise.

Usage:

```lua
MiniTest.expect.equality(x, y, { msg = "Invalid number of items" })
MiniTest.expect.no_equality(x, y, { msg = "Unexpected line counts" })
MiniTest.expect.reference_screenshot(screen, path, { msg = "Buffer not matching" })
```
@dimtion
Copy link
Author

dimtion commented Sep 21, 2025

Good call-out on the forward compatibility of opts I changed the interface in the last revision.

New usage:

MiniTest.expect.equality(x, y, { msg = "Invalid number of items" })
MiniTest.expect.no_equality(x, y, { msg = "Unexpected line counts" })
MiniTest.expect.reference_screenshot(screen, path, { msg = "Buffer not matching" })

Regarding expect.error and expect.no_error indeed the problem is backward compatibility. Hence, I did not wanted to complexify the pull request.

One way I was thinking was that error could check the first (or last) vararg type if it matches the schema we expect, and then use it as an opts arg. However, this still introduces ambiguity if the f callable also expect the same argument type.

@echasnovski
Copy link
Member

Regarding expect.error and expect.no_error indeed the problem is backward compatibility. Hence, I did not wanted to complexify the pull request.

One way I was thinking was that error could check the first (or last) vararg type if it matches the schema we expect, and then use it as an opts arg. However, this still introduces ambiguity if the f callable also expect the same argument type.

I am currently leaning towards making a breaking change for expect.error, but that would indeed require a lot more code changed. Let me think about this at least a couple of days, please, because I'd like to take time to make such decisions.

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

Successfully merging this pull request may close these issues.

2 participants