Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Some minor improvements. #16

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
3 participants
  • Add a new ASSERT-NIL assertion form. This is, of course, equivalent to ASSERT-FALSE. But names matter; without knowing your API in full detail I found myself wanting to write ASSERT-NIL to check that a list of results is NIL, but I had to use ASSERT-FALSE, which is (psychologically) not the same thing (even though for the purpose of evaluating values logically falsity = NIL).
  • Clean up the various specializations on the RECORD-FAILURE generic function. On CCL 1.9, this particular practice of using CALL-NEXT-METHOD was leading to errors (specifically, the list of applicable methods was changing because the first argument was going from a known symbol [via the EQL specializer] to an unknown symbol, i.e., the list of applicable methods was changing from a list of length 2 to a list of length 1.
  • In .gitignore, ignore fasls generated by a (64-bit) CCL.
  • Use HASH-TABLE-P to do some sanity checks on the tags and test tables.
  • Trim some unnecessary end-of-line whitespace.
Owner

ThomasHermann commented Mar 20, 2013

I need to remember to test on CCL, it seems to check the CLOS more than other implementations. With respect to the CALL-NEXT-METHOD in record-failure, the specialized versions should probably be :BEFORE methods. I want to experiment with CCL because as I understand the Hyperspec and Keene, there is no reason the original implementation should not have worked. I may be missing something though. Regardless, making the methods :BEFORE methods should correct the error.

With respect to returning NIL from package-table instead of signalling an error, why do you like that approach? I prefer signalling an error because then the user can handle it however they wish using HANDLER-BIND or HANDLER-CASE. I don't like returning NIL and having to check for a hash-table everywhere that package-table is called.

With respect to assert-nil, let me think about it. I'd like to avoid creep in the number of assert forms. Other test libraries don't even bother with multiple assert forms.

Regarding package-table, I see your point about conditions. I took this approach to solve the following problem: I evaluate (lisp-unit:list-tests) and get, unexpectedly, an error about hash tables (and similarly with lisp-unit:list-tags). This seems to me like an error in the library; I don't see why I, as a user of the library, should be on the lookout for the case when no tests are defined. In that case (so the thinking goes), lisp-unit:list-tests should just return NIL, i.e., the empty list (set), rather than signaling an error. This seems like a library-internal issue: "no tests defined" could mean "the test table has been initialized but is empty" or it could mean "the test table hasn't even been initialized". From the outside, though, my intuition tells me that this is a distinction without a difference.

One could define a condition and then, inside the library, use a handler-case in lisp-unit:list-tests (and lisp-unit:list-tags) to return NIL if the condition arises. (I guess I'm saying that I am not in favor of exporting this condition from the library, though it could be used internally.)

Owner

ThomasHermann commented Mar 20, 2013

Ah, okay, I think I understand the issue. I didn't spend much time experimenting with the changes for 0.9.5, been a little busy with other tasks. Give me some time to experiment with the behavior, I'll get it cleaned up.

Owner

ThomasHermann commented Apr 27, 2013

Everything except for ASSERT-NIL has finally been taken care of. Haven't had time to mull over ASSERT-NIL.

This needs rebasing to master.

Will this be merged in? It looks like it has some useful stuff. I could help getting it back up to speed.

Owner

ThomasHermann commented Sep 7, 2014

Everything has been added except for ASSERT-NIL. I continue to waffle on the utility of the assertion and want to avoid interface bloat. I need to make a decision and close out this pull request.

Owner

ThomasHermann commented Dec 12, 2015

Added ASSERT-NIL.

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