OdonataResearchLLC/lisp-unit

TAP output

Closed this issue · 13 comments

I had a Jenkins build server running and generating TAP files from lisp-unit tests using the (now removed) with-test-listener feature. Honestly I'm not sure where with-test-listener came from, I might have started with a lisp-unit fork.

I was wanting to add TAP support to the main lisp-unit repo, but am unsure of how to do so.

In my jenkin build, I implemented TAP support by adding a (defmethod asdf:perform :around ((o asdf:test-op) (system asdf:system)) ... ), in the Jenkins' .sbclrc. The code is messy: https://gist.github.com/3919013

Some of the key features of that implementation:

  • There is one TAP result for each failing assertion in a test - if there's a failure it's helpful to see what it was
  • There is one TAP result for ALL passing assertions in a test
  • TAP gets tricky about newlines and indentation
  • TAP files get generated without the individual lisp project having to know about it. I'm testing many different lisp systems with Jenkins, and it'd be a pain to go into each asd file and add duplicate code to make asdf:test-op write a TAP file

So, that said, I had some ideas for how to do this in lisp-unit:

  1. add a (write-tap-output test-results path) function. The current data in a test-results object is enough to make a simple TAP file of pass/fail, but does not store enough data about the testing process to list how many assertions passed in each test, nor would it support showing error messages in TAP output
  2. add a (with-tap-output (path) ...) that writes test results into a TAP file as tests are run. This would probably allow detailed errors without needing to store a bunch more stuff on test-result, and would also support my use case of having Jenkins listen in.

I think I like option 2 better, do you have any advice on how you'd like this implemented?

First, with-test-listener has been present since the original lisp-unit. I didn't think anyone was using it, sorry for causing this issue. The functionality it provided has been, for the most part, superseded by the test-report object.

With respect to how to get TAP support into lisp-unit, my recommendation is to add it as an extension file under the extensions directory, like 'tap.lisp' or 'test-anything-protocol.lisp'. Scanning over your code, I think the approach is going to be change the argument list of the internal generic function print-failure to include an argument for specifying the output format. Next, convert internal function print-error to a generic function with an argument for specifying the output format. Then, write the TAP methods for print-failure and print-error. Finally, write the macro with-tap-output that lexically sets some internal global variable like *output-format* to :tap and runs the tests. Maybe export *output-format* and negate the need to use with-tap-output, although that macro would be nice to have and should be available regardless.

I'm tied up this afternoon with work and will take a look at it tonight and this weekend.

Thanks for the feedback.

Tom

Thanks for the quick reply. I was also thinking the print-* functions might need another argument for what stream to print to (defaulting to t), or that might be another lexically bound global variable.

If I can find the time I'll fork and make a patch.

I did make a start on this, but didn't realize you had, too. https://github.com/AccelerationNet/lisp-unit/tree/TAP-output

I looked long and hard at the print-* functions, and to get really nice TAP output, you'd need to change print-error and print-summary as well to accept output-format and test-name, and that seemed like it might be tricky to thread that extra data through there.

I started on a different tack, adding a *signal-test-events-p* and a handful of conditions that might get signaled during the testing process: test-failure, test-error, and test-complete. This is closer to the with-test-listener approach, and I think it makes a lot of sense for extensions and will be a smaller change to lisp-unit.lisp.

I'll finish it off and then you can compare the two methods of extending output.

I haven't really made much progress with the TAP extension. I wanted to put a band-aid on the code to get the extension, but realized that it would be better in the long run to do some further cleanup of the internals. The biggest issue is how the internal-assert function records the results of the assertion. I'm working on an approach that creates an assert-result object that stores the detailed results of the assertion and storing lists of these objects in the tests-results object.

I'll look over your fork sometime in the next couple days and figure out how to synchronize.

Look at the TAP branch. I think most of the needed changes are there. I left that branch half done because I fell down the rabbit hole and started toying with fundamental improvements on storing and reporting results in internal-assert

I looked at the TAP branch, and to get the functionality I wanted there'd have to be a lot more arguments passed through the various functions; I think I saw the same rabbit-hole you did and opted for a signal-based approach. The low-level functions just announce what they did, and whoever cares can listen in via handler-bind. I think this is a nice way to keep extensions know what's going on, and allows for multiple extensions listening in on what's happening.

I just finished up TAP support in my branch: https://github.com/AccelerationNet/lisp-unit/tree/TAP-output

Thinking about it, the conditions I'm signalling are pretty close to assert-result objects, you might be able to have %run-test-thunks listen for those signals and store the condition objects in the test-results.

If you want to store ALL test details, then I'm thinking:

  • creates classes similar to my test-failure and test-error conditions in AccelerationNet/lisp-unit2@6c47d21
  • rename test-results to test-run-summary (or some such)
  • create a test-result class that represents the outcome of one named test (likely returned value from record-result) that stores the number of passed assertions and lists of test-error and test-failure
  • add a list of test-results to test-run-summary

I think that could all be done on top of my signalling system pretty easily; record-result could listen for test outcome signals and accumulate the results.

Also, if all the details are on the test-results, than the TAP extension gets a lot simpler, it can just accept a test- run-summary object and iterate over it, it doesn't need to listen in on the process.

I'll look into this in more detail when I get a chance, but I think we are headed in the same direction.

Check out the assertion-records branch. I took the route of creating objects for all test results and returning that. Look at the generic functions print-failure and print-error. You should be able to just write a print-tap generic function to work off of the results objects.

I'm having second thoughts about storing comprehensive results of all tests in a results database. Running the unit tests take a long time and conses a lot. Maybe a little profiling will help identify whether storing comprehensive results can be made efficient. May need to consider implementing the signals approach.

I've been using the comprehensive results for several days now and it actually isn't that big of a performance hit. Take a look at it and if it works, I'll merge it into master for 0.9.3.

Compared to whatever the tests are actually doing, I wouldn't expect the bookkeeping to be a significant performance hit. If it becomes an issue it should be pretty easy to add a new flag controlling whether to record accumulate the details or not.

I looked over the assertion-records branch, it looks pretty good to me. I'll make a new branch based on that and rework my TAP extension to export 2 functions that will meet my needs:

  • (defun write-tap (test-results-db pathname))
  • (defmacro with-tap-output (pathname))

I've merged the test results database into master and tagged it 0.9.3. As soon as you have this extension finished, I'll merge it in and delete the assert-records and tap branches.

It may be a little late, but I think it would be better to pass a stream to write-tap rather than a pathname.

(defun write-tap (test-results-db &optional (stream *standard-output*)))

See #10, this request is now invalid.