sassoftware / jsl-hamcrest

Implementation of Hamcrest for JSL.
Apache License 2.0
11 stars 8 forks source link

Reporter improvements #52

Open VDFaller opened 5 years ago

VDFaller commented 5 years ago

I'd like to see timing and module in the reporters as well.

Timing

Timing would allow us to see if performance changes across different runs. I think having an HPTime() on start and end of uttestcase() and the utassertthat() would help us see performance differences. I'm probably missing some places.

Module

Also I was thinking about adding on each of the reporter add X() functions. For instance,

// Function: add failure
add failure = Method( {label, test expr, description, mismatch, lre, payload=Empty()},
    file_name = Include File List()[1];
    record = Eval List( {label, Name Expr( test expr ), description, mismatch, lre,  Name Expr( payload ), file_name} );
    Insert Into( this:failures, Eval List( {record} ) );
    0;
);

I'm not sure what impact that would have elsewhere though.

emccorkle commented 5 years ago

Could you elaborate on your add failure example? Is it just the addition of file_name? And which specific Reporter are you considering modifying? If you don't have a specific Reporter in mind, one option to consider here might be similar (in idea) to the UtWindowDispatchingReporter but using Include File List instead of Current Window.

As for timing, there are a couple of places where we have started down this road. One is the ut elapsed time Matcher. Another is the UtTimingReporter. Going forward, I see several options.

The last two options are likely general enough to include directly in jsl-hamcrest. For all the options, unless you have a "correct" or "baseline" timing, there can really be no assertion on the current timing. For that reason, the current timing either needs to ride in payload with an assertion or (more likely) it needs to be given a new Reporter method entirely. Or maybe even an entirely new type of reporter object (with no add failure, etc) depending on if the benchmarks can also have assertions.

VDFaller commented 5 years ago

Sorry, yeah I was thinking generically. Basically the goal is to get the reporter to know which file/uttestcase/uttest/utassert was run for every ut assert that that is made, so that it is easier and faster to trace CI/CD Failures for large projects.

And for timing, I was thinking for just benchmarking so that we can watch timings of uttests and if they suddenly increase (between JMP versions or commits), that we can get a warning. I could do the Setup and Teardown on all of our cases, but I think having the all reporters know the timings of their tests would be useful.

All of this we're writing into the custom reporter I'm working on #56, but I figured if it was something that other reporters could use, put it in here first.

BenPH commented 3 years ago

I'm interested in adding the file that the tests came from like @VDFaller mentioned. Specifically, to the JUnitXMLReporter so it would have <testcase name="test1" file="MyTest.jsl" ...>. Since the JUnit reporter is based on the CollectingReporter I can see two ways to get the file name while the results are being collected:

  1. Vince's method above. Adding the file name to the record list in the add failure, add success, add unexpected throw methods.
  2. Include the file (optionally) in ut concat test label so that the output could be file name > test case name > test name > label and we could parse it out in the reporter. This way file name could be available to more than just the JUnitXMLReporter.

@emccorkle Do you think either of these make sense or do you see any alternatives?

emccorkle commented 3 years ago

I think modifying UtCollectingReporter to collect file info via Vince's method (1) would be fine. Then modifying the reports of UtCollectingReporter and JUnitXMLReporter to use that info. That should be a pretty low impact change.

BenPH commented 3 years ago

Thanks @emccorkle. I've made this change and various other improvements to the JUnitXMLReporter in #110.