assertions need refactoring of output

[Tieske]

Currently an assertion takes 2 parameters

1. state
2. list of arguments, with an additional n field to indicate the number of elements (to take care of nil values)

It returns 1 value; a boolean being true if the assertion passed

On the side; the in coming arguments list is modified to prepare the arguments for proper display in the output. So actually it returns a second argument, through its second parameter (passed by reference).

imo it should actually get 3 parameters, the last being a (shallow) copy of the arguments table. And it should return 2, the boolean assertion result and an output table, being the modified copy of the arguments table.

[ajacksified]

Yeah. So, I'm checking doing something like:

local s = spy.new(function() return 1,2,3 end)
fn()

assert.spy(s).was.returned_with(1,2)

The problem is, (and we have this problem for called_with too) the output formatters return the arguments list in the formatter, so:

"Expected arguments %s but got %s"

becomes

"Expected arguments 1 but got 2"

When it should be

"Expected arguments 1,2 but got 1,2,3"

I'm thinking that perhaps we could add "expected" and "actual" onto state, and refactor to use mustache (or even plain old gsub). So the new string would be:

"Expected arguments {{actual}} but got {{expected}}"

This would allow more flexibility for tests that aren't testing arguments against each other, but are instead testing arguments against some internal list. We would need to make a generic way to turn an object into a string, which I think is just a tweak of the existing formatters.

[Tieske]

I like the 'named' placeholders. That should be doable, also easier, as currently sometimes the 'expected' and 'got' get reversed. By naming you'll prevent that.

What happens now is that the arguments are passed to the assertion, which modifies them for display. If you don't make the shallow copy I mentioned above, but let the assertion create its own list, keyed by placeholder name, that would work the same way.

The argument list has a subtable now, 'nofmt' that has under the same key a truthy value that prevents the output being formatted again. I think that should remain in place, so generic formatting of arguments is done as processed in the formatter list (and possibly changed by the user), and when set, the assertion basically overrules the formatters.

There is a special case though, the nil values. You cannot set named elements to nil as in outputlist["expected"] = nil. The n value in the list served this purpose, but that only works with lists. A workaround might be to create a shallow copy anyway, and attach a metatable that changes nils into some predefined constant.

regarding the assertion returned_with; does it also keep track of all return values (in case of multiple calls) or just the last call? If it remembers all calls (like called_with does), then what will you output as {{actual}} when the assertion fails?

[ajacksified]

It works like called_with and keeps track of all. Good question - perhaps it should return nothing for {{actual}} and the statement can be reformatted to display only expected. Or..

If we use mustache, we can format it like: "Expected arguments\n{{#actual}}{{#values}}{{.}}{{/values}}\n{{/actual}} but got {{#expected}}{{#values}}{{.}}{{/values}}{{/expected}}"

It will make all of our statement more verbose, but way more flexible. This means that in the called_with and returned_with functions, we'll need to make a status.actual.values table which is an array of formatted strings; so display is controlled from the assertion and the output string rather than any formatters within assert.lua.

[Tieske]

so display is controlled from the assertion and the output string rather than any formatters within assert.lua.

This means that every assertion has to find its own way to format arguments? That is not flexible at all. The current formatters provide the ability to add a custom formatter for a specific object/table or userdata (verified by its metatable for example). That will be lost if you convert to strings in the assertions.

otoh; the current assert:format() function formats a list of arguments, if we change that to format an individual value, then the assertions can call assert:format() for each argument to be included in the output.

[ajacksified]

Well, the assertion will need to say which value is expected and which is actual anyway; this seems like a slight extension of that. Maybe instead we make a call like status:actual:add(thing) for actual, status:expected:add(other_thing)? And then status can be passed into mustache. Now all they have to do is know what was expected and what was actual, which should be a reasonable assumption.

Also, that mustache string can be condensed, if actual and expected are tables acting like arrays:

"Expected arguments\n{{#actual}}{{.}}\n{{/actual}}\nbut got {{#expected}}{{.}}{{/expected}}" "Expected to be equal:\n{{#actual}}{{.}}\n{{/actual}}" etc

[Tieske]

That should work. The assertion comes with its own success and failure templates anyway, so it knows whether 1, 2, or x number of elements are needed (and what they are called) in that template. It can prepare those, and for formatting 'stuff' it can fallback on the generic formatters to turn any value (including nils) into a string basically.