lunarmodules / luassert

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

Fix stripping of file info for errors that refer to multiple files/lines #122

Closed kcarl-adb closed 9 years ago

o-lim commented 9 years ago

Doesn't Lua only throw errors with only one file/line (at the beginning of the message)? The only way to get multiple files/lines would be to catch and rethrow, right? In this case, wouldn't the extra files/lines, technically, be part of the error message being thrown?

-- myfile.lua
local myfunc()
  error('some error')
end
local func()
  local ok, err = pcall(myfunc)
  if not ok then
    error(err) -- err would start with "myfile.lua:3: "
  end
end

So when you do assert.has_error(func, msg), you are verifying that msg is equal to what was passed to error, which in this case would be "myfile.lua:3: some error". So if you remove multiple occurrences of file/line, then you are no longer checking for a string passed to error, but a sub-string passed to error, in which case error_matches (which checks against a Lua pattern) might be more appropriate.

scouten commented 9 years ago

@o-lim: Technically, yes, you are correct. IMHO the has_error method becomes effectively useless if you have any intervening catch/rethrow wrapper in place. This could easily change over the lifetime of the software being tested, thus rendering existing tests invalid merely due to an implementation detail. I'm not sure that's a desirable result.

mpeterv commented 9 years ago

@scouten there is no single way to wrap errors AFAIK. You can concatenate error messages, with or without some labels like "original error: ", you may or may not append original stacktrace, you can use a table instead of a string, etc. So IMHO it makes sense for has_error to only handle the single "standard" error format.

On master has_error returns the error on success, so you can use it to strip the error in the way you want:

local err = assert.has_error(f):gsub(".-:%d+: ", "")
assert.matches("original error", err)
kcarl-adb commented 9 years ago

agreed. closing request.