Fix bug in assertTableEq

[jucor]

Hi guys

assertTableEq fails to distinguish between table A and B if the A is shorter than B but matches on all its terms. The attached patch uses recursive comparison by double-inclusion to make sure the two tables actually match (recursively).

I also included tests to avoid regressions.

[clementfarabet]

Great, thanks for that patch.

I've started using Penlight for quite a few things now. His tablex package in particular, includes lots of nice tools that are missing from Lua, including different types of deep copies and deep comparisons.

[jucor]

Glad to help :-) I note Penlight for future uses (since my recursive comparison is quite crude, will break for too deeply-nested tables, but that's good enough for now).
I keep stumbling upon testing situations where porting our asserts into a full-fledged framework (e.g. lunitx) could be useful.
Case in point here: testing that a test fails for a certain input (see the TODO line in the unit tests I've added), which is not yet feasible in Tester without jumping through hoops, because the core assert_sub() method do more than just asserting: it updates the Tester internal counts. I think that a design like the original jUnit did not use a modified assert but had a higher caller catch the errors thrown: that higher caller would then update the internal counters. Checking for an assert to fail would thus be a simple call to assertError(the_assert_supposed_to_fail) -- which we can't do now, since the_assert_suppose_to_fail would be added to the Tester's list of failed asserts.
I'll see with Koray if he still thinks a more heavyweight suite is not needed.
Cheers,

[koraykv]

I think there is nothing stopping us from using any other test suit and any other suggestions to improve tester is more than welcome too. But at this point, I think completely switching to an outside test suite is not the right thing to do. We have only been supporting luarocks packages for a short while. I want to see how that works out for many cases before making torch and nn depend on anything else.

[clementfarabet]

I agree, let's take more time. But eventually we won't have a choice: a
full standard test system would be really useful, and increase confidence
each time we fold a new version.

On Sunday, December 9, 2012, koray kavukcuoglu wrote:

I think there is nothing stopping us from using any other test suit and
any other suggestions to improve tester is more than welcome too. But at
this point, I think completely switching to an outside test suite is not
the right thing to do. We have only been supporting luarocks packages for a
short while. I want to see how that works out for many cases before making
torch and nn depend on anything else.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-11175764.

Clement