Problem/Motivation

An initial implementation of a code standards check has been made in #1266444: DrupalCI should return failed for poor code style - php. We need to move forward and prepare the test results for integration with drupal.org.

Proposed resolution

This was proposed by @Mixologic in IRC:

We need to get the coding standards output into a format that Jenkins can consume, so that we can have something that drupal.org can see. PHP CodeSniffer should be configured to use the 'CheckStyle' output format. There is a Jenkins plugin that can consume it directly (which in turn means that there is a JSON API endpoint for that data from Jenkins). The output should end up in the Jenkins working folder on the testbot: /var/www/html/artifacts inside the container.

There is already code present that does a similar task: the JunitXMLFormat build step converts the Simpletest results into a JUnit XML for Jenkins.

Follow ups / future plans

For the moment it is not desirable to add the coding standards check as a test target because that would require us to spin up an entire new bot just to do the standards check. In the future this might be a possibility because the coding standards check is very lightweight - it does not even need a running database, so it could in theory run on a small or micro instance in parallel with the other tests.

Once we've got some output to work with on the dispatcher, then we'll need to work on project_issue_file_test repository to gather the results and display them on drupal.org.

The easiest way to then get an automated test up and running is to add the new php_codesniffer task to the already existing Jenkins job. Initially the goal is to make it review patches only, so that we get a "per commit report" of code violations. When this stabilizes we can move to per-branch tests.

To speed up test results it would be great to only test files that are actually changed, but this will require some improvements to PHP CodeSniffer.

Comments

pfrenssen created an issue. See original summary.

jthorson’s picture

What is the current status of the 'production' branch ... has the dev work been merged in yet?

The 'improved patch process' added the ability to tell which files had been modified, by checking $job->getJobCodeBase()->getModifiedFiles() (or something similar). But this might still be floating around in the -dev branch.

pfrenssen’s picture

I have some experience with scanning only modified files using a git pre-push hook to scan changed files before pushing, but this is not reliable. If you pass a list of files to PHP CodeSniffer it ignores the exclude rules that are defined in the ruleset. This would cause failures if for example a composer dependency is updated, or if a change to any non-PHP file is made.

It would be impractical to solve this on our side since we would need to fully parse and understand the ruleset format. This is something that we can try to fix in PHP CodeSniffer itself.

Mixologic’s picture

Component: Code » Jobs and Job Handling
Mile23’s picture

Status: Active » Closed (duplicate)