lunarmodules / luassert

Assertion library for Lua
MIT License
202 stars 77 forks source link

Add error_matches assertion #113

Closed o-lim closed 9 years ago

o-lim commented 9 years ago

This adds an assertion similar to has_error, but matches the error with a Lua pattern or substring instead of an exact string.

mpeterv commented 9 years ago

I think it would be more flexible if has_error returned error message on success instead. So that you could use it without error argument to ensure there is an error and capture it, and then use other assertions to validate it. In general, successful assertions should return whatever they checked, IMO, similarly to standard library assert.

o-lim commented 9 years ago

@mpeterv I liked your suggestion of asserts returning their arguments on success, so I submitted PR #115 to do just that. This mirrors the standard Lua assert.

assert (v [, message])

Issues an error when the value of its argument v is false (i.e., nil or false); otherwise, returns all its arguments. message is an error message; when absent, it defaults to "assertion failed!"

mpeterv commented 9 years ago

@o-lim simply returning all arguments works for many assertions, e.g.

local res = f(x)
assert.table(res)
-- Some custom validation of res

becomes more concise

local res = assert.table(f(x))
-- Some custom validation of res

but it is not helpful for has_error, because the value you may want to continue validating (the actual error) is not an argument, but some intermediate value that is lost on success. I'd like to be able to do something like this:

local err = assert.has_error(f)
-- Some custom validation of err (table)

which currently requires manual work with pcall. So, some assertions need a way to specify what to return on success. Perhaps another special field in the arguments table passed to assertion callback could be used for that? For example, if an assertion set args.returns = {"foo", n = 1} it would return "foo" on success. And if the field is missing, original arguments could be returned by default, as you implemented. What do you think?

o-lim commented 9 years ago

@mpeterv I agree with you. I realized the same thing for has_error after I pushed the PR, but it was getting late so I just left it. I'll update the PR later today when I have a bit more time.

o-lim commented 9 years ago

Updated the PR to take a function as the first argument to be consistent with has_error.

thibaultcha commented 9 years ago

That sounds great! As well as #115 too!