Improving Junit-style reporting in PHP CodeSniffer Pipelines

Posted on Sat 16 May 2020 • Tagged with Work, TimeTac

Recently I was shown that in a GitLab-based pipeline I had designed and implemented, the results were presented poorly. Here is an example of how this looked like.

# example output from failing pipeline

Running before_script and script
 $ mkdir -p test-reports
 $ phpcs --version
 PHP_CodeSniffer version 3.5.4 (stable) by Squiz (http://www.squiz.net)
 $ phpcs --basepath="$PWD" --parallel=$(nproc --all) --report=junit --report-file=test-reports/php-codesniffer.xml "$PWD"
 Time: 21.21 secs; Memory: 7.25MB
Running after_script
Uploading artifacts for failed job
 Uploading artifacts...
 test-reports/*.xml: found 1 matching files         
 [...]
 ERROR: Job failed: exit code 1

From this output, it was clear that the job failed, but there was no immediate hint why it failed. That’s because we were writing the report as XML to the specified file - and only there. Here’s how the pipeline definition for this step looked like.

# fragment from .gitlab-ci.yml

PHP Coding Standard:
  stage: fast checks
  image: timetac/ci:php5.6
  script:
    - mkdir -p test-reports
    - phpcs --version
    - phpcs
      --basepath="$PWD"
      --parallel=$(nproc --all)
      --report=junit
      --report-file=test-reports/php-codesniffer.xml "$PWD"
  artifacts:
    reports:
      junit: test-reports/*.xml
  tags:
    - docker
    - timetac

Improving Display of Details on stdout

After some research I found that phpcs, the tool we’re using to enforce our PHP coding standard is capable of generating multiple reports in one run. I slightly modified the pipeline instructions:

# changes to .gitlab-ci.yml

-      --report=junit
-      --report-file=test-reports/php-codesniffer.xml "$PWD"
+      --report-full
+      --report-junit=test-reports/php-codesniffer.xml "$PWD"

I instructed phpcs to generate both reports and then made the check fail on purpose to see example output:

# example output from pipeline with adjusted report display

Running before_script and script
 $ mkdir -p test-reports
 $ phpcs --version
 PHP_CodeSniffer version 3.5.4 (stable) by Squiz (http://www.squiz.net)
 $ phpcs --basepath="$PWD" --parallel=$(nproc --all) --report-full --report-junit=test-reports/php-codesniffer.xml "$PWD"
 FILE: cli-config.php
 ----------------------------------------------------------------------
 FOUND 1 ERROR AFFECTING 1 LINE
 ----------------------------------------------------------------------
  18 | ERROR | [x] Concat operator must not be surrounded by spaces
 ----------------------------------------------------------------------
 PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
 ----------------------------------------------------------------------
 Time: 14.85 secs; Memory: 7MB

While waiting for the results of this run, I made some screenshots how our development team can find the parsed Junit results in GitLab’s web interface. They are surfaced in the Tests tab of a Pipeline (in addition to being displayed directly in Merge Requests if you happen to use GitLab for both Continuous Integration and Development).

Screenshot of the Tests tab missing classname and barely readable test name

Improving Display of Results in GitLab

However, as evident in the screenshot, the results weren’t presented as nicely as they could be. This sent me quite far down the PHP rabbit hole. After some research I understood that the collected XML file was missing the classname attribute. I checked the issue link to see that GitLab instead of customizing their parsing fixed the issue upstream instead, which is commendable.

References:

Well, let’s also do that. On the other hand, who knows how long it takes for a PR to be accepted and merged? Perhaps a custom report is an option while I wait for upstream to accept possible contributions.

I pinged one of the developers of phpcs via Twitter and got a reply from a project contributor, that yes, it is possible to generate a custom report. This was not evident in its documentation. To use a custom report type, you need to ask for a relative path, an absolute path or a Fully Qualified Function (FQN) in the --report parameter.

Example:

# call to phpcs requesting a custom report type

phpcs \
  --basepath="$PWD" \
  --parallel=$(nproc --all) \
  --report="$HOME/Repositories/PHP_CodeSniffer/src/Reports/Junit.php" \
  --report-file=testreport.xml \
  "$PWD"

It is also possible to request this inside the .phpcs.xml configuration for your project.

<!-- fragment requesting a custom report of type "WPQA" -->

<arg name="report" value="WPQA"/>

References:

The next step in my adventure was to investigate how the Junit report was generated at the moment and hoping it can be understood easily. Simple enough, I found Junit.php which looked straightforward on the first glance.

On a more closer inspection I found some things I was dumbfounded by.

Making indention consistent

The first thing that struck me as odd was that in generateFileReport() the XML is produced by building it step by step via XMLWriter and its methods while in generate() the beginning of the document is written out via echo calls that contain the actual XML-formatted string. I initially investigated this because I was wondering why the final XML report uses correct indentation for the first elements and then seems off for the bulk of the file.

// original snippet generating report.xml

<?php // This opening tag is only for syntax highlighting with pygments
echo '<?xml version="1.0" encoding="UTF-8"?>'.PHP_EOL;
echo '<testsuites name="PHP_CodeSniffer '.Config::VERSION.'" errors="0" tests="'.$tests.'" failures="'.$failures.'">'.PHP_EOL;
echo $cachedData;
echo '</testsuites>'.PHP_EOL;

<!-- inconsistently indented fragment of report.xml -->

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="PHP_CodeSniffer 3.5.4" errors="0" tests="104" failures="1">
<testsuite name="tests/unit/MiscUtilTest.php" errors="0" tests="1" failures="0">
    <testcase name="tests/unit/MiscUtilTest.php"/>
</testsuite>

In trying to make this uniformly written using calls intended for writing XML, I learned that between XMLWriter, SimpleXML, XMLReader and DOM, it can be tricky to find the correct solution for your problem. In my specific case, the only solution that worked for accepting and later properly formatting partial, invalid XML fragments was DOM. However, this attempt at making the final result prettier comes with downsides in readability.

// new snippet generating report.xml

<?php // This opening tag is only for syntax highlighting with pygments
$dom = new \DOMDocument();
$dom->formatOutput       = true;
$dom->encoding           = "UTF-8";
$dom->preserveWhiteSpace = false;

$testsuites = $dom->createElement("testsuites");
$testsuites->setAttribute("name", 'PHP_CodeSniffer '.Config::VERSION);
$testsuites->setAttribute("errors", 0);
$testsuites->setAttribute("tests", $tests);
$testsuites->setAttribute("failures", $failures);

$fragment = $dom->createDocumentFragment();

/*
 * Using XML that is partially formatted in appendXML() results in
 * dom->formatOutput ignoring the fragment during formatting.
 */

$fragment->appendXML($cachedData);

$testsuites->appendChild($fragment);
$dom->appendChild($testsuites);

// Saving and loading the string forces pretty formatting.
$tmp = $dom->saveXML();
$dom->loadXML($tmp);
echo $dom->saveXML();

<!-- consistently indented fragment of report.xml -->

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="PHP_CodeSniffer 3.5.4" errors="0" tests="104" failures="1">
  <testsuite name="tests/unit/MiscUtilTest.php" errors="0" tests="1" failures="0">
    <testcase name="tests/unit/MiscUtilTest.php"/>
  </testsuite>

The upside of this approach was that I was able to keep generateFileReport() exactly as it is. Since I could not figure out what the point to having partial results as a string that differs for every report type is, I was happy to not have to change this function. A nice side effect of consistent indention is code folding working with more editors.

Implementing missing attributes

The second issue I was facing is that for GitLab to display the test results properly, several attributes were missing and some were formatted in a way that were less useful than they could be. Since GitLab are typically quite thorough at requiring tests for how their software should behave, it was helpful to look at the rspec for this behavior to find which XML attributes are supported.

References:

# snippet from GitLab's Junit rspec

expect(test_case.classname).to eq('Calculator')
expect(test_case.name).to eq('sumTest1')
expect(test_case.execution_time).to eq(0.01)
expect(test_case.status).to eq(status)
expect(test_case.system_output).to eq(output)

Given the above list, classname was missing. When checking the Junit fixtures found in GitLab’s code, it seemed file was also missing. I went to verify this theory by breaking some files in the scanned code base on purpose.

# call to phpcs after putting coding standard violations into 2 files

phpcs --basepath="$PWD" --parallel=$(nproc --all) --report-full "$PWD"
FILE: tests/unit/DeploymentCommandsTest.php
------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------
 44 | ERROR | [x] Space after opening parenthesis of function call prohibited
 44 | ERROR | [x] Space after opening parenthesis of function call prohibited
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------


FILE: cli-config.php
------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
------------------------------------------------------------------------------
 18 | ERROR | [x] Concat operator must not be surrounded by spaces
 19 | ERROR | [x] Concat operator must not be surrounded by spaces
 77 | ERROR | [x] Space after opening parenthesis of function call prohibited
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------

The generated report.xml was missing the attributes that could make our lives easier.

<!-- fragment of report.xml after inserting coding standard violations -->

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="PHP_CodeSniffer 3.5.4" errors="0" tests="107" failures="5">
  <!-- ... -->
  <testsuite name="tests/unit/DeploymentCommandsTest.php" errors="0" tests="2" failures="2">
    <testcase name="PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket at tests/unit/DeploymentCommandsTest.php (44:35)">
      <failure type="error" message="Space after opening parenthesis of function call prohibited"/>
    </testcase>
    <testcase name="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket at tests/unit/DeploymentCommandsTest.php (44:35)">
      <failure type="error" message="Space after opening parenthesis of function call prohibited"/>
    </testcase>
  <!-- ... -->
  <testsuite name="cli-config.php" errors="0" tests="3" failures="3">
    <testcase name="Squiz.Strings.ConcatenationSpacing.PaddingFound at cli-config.php (18:17)">
      <failure type="error" message="Concat operator must not be surrounded by spaces"/>
    </testcase>
    <testcase name="Squiz.Strings.ConcatenationSpacing.PaddingFound at cli-config.php (19:16)">
      <failure type="error" message="Concat operator must not be surrounded by spaces"/>
    </testcase>
    <testcase name="PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket at cli-config.php (77:50)">
      <failure type="error" message="Space after opening parenthesis of function call prohibited"/>
    </testcase>
  </testsuite>

As expected, there was neither file nor classname. Furthermore, the line and column indicator should likely be moved to message in failure for easier remediation. Taking the first failure as an example, it should probably look like this:

<testcase name="PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket" classname="DeployCommandsTest" file="tests/unit/DeploymentCommandsTest.php">
    <failure type="error" message="Space after opening parenthesis of function call prohibited (line 44, column 35)"/>
</testcase>

The next block is what the final XML structure looks like, after some iterations. I wanted to completely remove the line:column part from name but realized that some consumers of the report might not be able to cope well with testcase elements that have both the same classname and name. I used the Pull Request to eslint as template for how to implement the additional attributes.

<testcase classname="DeploymentCommandsTest" file="tests/unit/DeploymentCommandsTest.php" name="PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket (44:35)">
  <failure type="error" message="Space after opening parenthesis of function call prohibited (line 44, column 35)"/>
</testcase>

Here’s what the difference looks like when viewed in GitLab. Screenshot of the Tests tab with classnames, almost readable test names as well as line and column numbers in the error message

Given that, I have adjusted Junit.php and sent a Pull Request to phpcs.

Make it work right now

While I wait for this part to be merged, I’ve manually patched it into the phpcs installation we use during testing by overwriting the file.

# fragment of .gitlab-ci.yml
  script:
    # manually copy custom report. It's not possible to print a custom report to a file
    # AND a builtin one to stdout in the same run.
    # remove after https://github.com/squizlabs/PHP_CodeSniffer/pull/2964 is released
    - cp resources/scripts/ci/Junit.php /opt/vendor/squizlabs/php_codesniffer/src/Reports/Junit.php
    - phpcs --version
    - phpcs
      --basepath="$PWD"
      --parallel=$(nproc --all)
      --report-full
      --report-junit=test-reports/php-codesniffer.xml "$PWD"

Dismantling a Leviathan is a series about improving tests and automated quality tools on a large existing codebase.

Improving Junit-style reporting in PHP CodeSniffer Pipelines is part 1 of Dismantling a Leviathan:

  1. Improving Junit-style reporting in PHP CodeSniffer Pipelines