garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.78k stars 603 forks source link

Request: extend JUnit report #1120

Open jimivdw opened 4 years ago

jimivdw commented 4 years ago

I've recently added the JUnit report to my (GitLab) CI pipeline. It works great, but I'd like for it to provide a bit more info on which exact test is which and what is failing (in case of a failure).

Current output:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="BackstopJS" tests="3" failures="1" errors="1" skipped="0">
    <testcase classname="mySelector" name=" ›› MyLabel">
      <failure message="Design deviation ›› MyLabel (mySelector) component"/>
      <error message="Design deviation ›› MyLabel (mySelector) component"/>
    </testcase>
    <testcase classname="mySelector" name=" ›› MyLabelWithDifferentViewports"/>
    <testcase classname="mySelector" name=" ›› MyLabelWithDifferentViewports"/>
  </testsuite>
</testsuites>

I'd like to see the following additions:

The output could then look something like:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="BackstopJS" tests="3" failures="1" errors="1" skipped="0">
    <testcase classname="mySelector" name="MyLabel ›› mySelector ›› myViewport">
      <failure message="Design deviation (0.42%, w: 3, h: 14) ›› MyLabel ›› mySelector ›› myViewport"/>
      <error message="Design deviation (0.42%, w: 3, h: 14) ›› MyLabel ›› mySelector ›› myViewport"/>
    </testcase>
    <testcase classname="mySelector" name=" MyLabelWithDifferentViewports ›› mySelector ›› myViewport"/>
    <testcase classname="mySelector" name=" MyLabelWithDifferentViewports ›› mySelector ›› myOtherViewport"/>
  </testsuite>
</testsuites>

I'd be happy to submit a PR for this if that would speed up things. I could also add configuration toggles for some/all of these changes if you'd like to keep the current default.

garris commented 4 years ago

@jimivdw thanks for the suggestions here. I personally haven't used the jUnit reporting -- it your idea is reasonable but I wonder if adding this extra data might break existing implementations. Would it be possible to include this data using a different properties (or child tags) to avoid collision? Another alternative might be to provide a jUnit reporting template like we do for docker configs (see dockerCommandTemplate in the documentation for an example)

masi commented 4 years ago

Is it possible to write a custom reporter? I couldn't find a configuration option.

garris commented 4 years ago

Curious what the use-case is for custom reporter.

masi commented 4 years ago

@jimivdw asked for quite specific changes. Your idea of templates sounds reasonable. To satisfy the needs of the most extraordinary manner a custom reporter is the most flexible solution.

For my needs in GitLab CI I am content with . I assume the duplication with is for other consumers. What GitLab CI needs though is a text content of . It ignores the message attribute (there are open tickets with no activity).

I wonder why the OP didn't ask for the URL or the path section of the URL to be included.

Anyway, if one can set the complete element with an ample supply of variables I think most junit report users will be content.

deap82 commented 3 years ago

I'd really also like to see some improvements here. This is what the current report looks like in our Azure DevOps CI:

image

As you can see down to the left there are many test labels that look identical due to not having selectors and viewports included. I can't imagine the changes suggested by @jimivdw would break anything existing.

Also, there is an "Attachments" tab in the details pane to the right. Imagine how awesome it would be if reference, test and diff images could be displayed there 😍. Haven't looked in to it though, but I guess it would be possible.

garris commented 3 years ago

Oh sure. I am totally fine with enhancements here as long as they are additive. @deap82