Search

Sunday 14 April 2013

Sirius: Adding code style checks

We already set up the build process for Sirius as well as we reserved the placeholders for testing. So, further improvement in this area is to extend build with different tests and checkpoints which make more detail verification. This time we'll add more modules which perform static testing for our components and they should fail the build if static checks didn't pass (especially taking into account that it was initially planned).

What would we look for? Basically that's:

  • Conformity to coding standards - all code standards are made to provide unified approach of writing code as well as reading and maintaining it.
  • Various cases of improper code usage - a lot of actual application errors are results of improper code use or use of potentially dangerous code constructions. So, the earlier we find them the less number of silly mistakes we take during further testing stages
Actually, we've made first approximation for that while we checked the compilation ability. If system isn't compilable it's not working at all. So, we're just making more detailed checks.

Since Sirius modules use different programming languages the tool set will be varying based on that. We'll use the following tool set for that:

LanguageEngine
Java
C#
Ruby
OK. Let's see how it would be added.

Java modules static testing

Installation

Both PMD and Check Style are provided as Maven plugins. So, we can use them within our Maven project. So, first step to enable both PMD and Check Style we should add Maven entry like:

 <reporting>
  <plugins>
   <plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>2.9.1</version>
   </plugin>
   <plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <version>3.0.1</version>
   </plugin>
   <plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-jxr-plugin</artifactId>
    <version>2.3</version>
   </plugin>
  </plugins>
 </reporting>
Thus we enable the reports genertion for both systems.

Configuration

Next step is to configure some specific options for PMD and Check Style. In my example I'll make settings which enable code analysis during verify stage so e.g. it will be executed for sure during release process. For this purpose I should update build section of pom.xml file with the following settings:

 <build>
  <plugins>
   ..........
   <plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <version>3.0.1</version>
    <executions>
     <execution>
      <goals>
       <goal>check</goal>
       <goal>cpd-check</goal>
      </goals>
     </execution>
    </executions>
   </plugin>
   <plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>2.9.1</version>
    <executions>
     <execution>
      <goals>
       <goal>check</goal>
      </goals>
     </execution>
    </executions>
   </plugin>
   ..........
  </plugins>
  ..........
 </build>
As it's seen from the above example we're not only adding plugin entries we're also specify tasks to execute. They will be performed during verify stage.

Usage

That's basically it. Now we can make the above changes to the pom.xml and run the command line like:

mvn clean verify
That will perform code analysis. In case you have some errors the output would look like:
    [INFO] >>> maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client >>>
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:pmd (pmd) @ sirius.java.client ---
    [INFO] 
    [INFO] <<< maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client <<<
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] >>> maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client >>>
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:cpd (cpd) @ sirius.java.client ---
    [INFO] 
    [INFO] <<< maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client <<<
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] --- maven-checkstyle-plugin:2.9.1:check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] There are 1 checkstyle errors.
    [INFO] ------------------------------------------------------------------------
    [INFO] BUILD FAILURE
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time: 3:33.444s
    [INFO] Finished at: Sun Apr 07 14:49:42 BST 2013
    [INFO] Final Memory: 17M/41M
    [INFO] ------------------------------------------------------------------------
    [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.9.1:check 
 (default) on project sirius.java.client: You have 1 Checkstyle violation. -> [Help 1]
    [ERROR]
and that would fail the build. Once all errors are fixed you'll see the output like:
    [INFO] >>> maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client >>>
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:pmd (pmd) @ sirius.java.client ---
    [INFO] 
    [INFO] <<< maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client <<<
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] >>> maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client >>>
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:cpd (cpd) @ sirius.java.client ---
    [INFO] 
    [INFO] <<< maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client <<<
    [INFO] 
    [INFO] --- maven-pmd-plugin:3.0.1:cpd-check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] --- maven-checkstyle-plugin:2.9.1:check (default) @ sirius.java.client ---
    [INFO] 
    [INFO] ------------------------------------------------------------------------
    [INFO] BUILD SUCCESS
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time: 2:57.938s
    [INFO] Finished at: Sat Apr 13 13:45:33 BST 2013
    [INFO] Final Memory: 17M/41M
    [INFO] ------------------------------------------------------------------------
Additionally the target folder will contain the analysis report with the analysis results. It's typically named as checkstyle-result.xml

Integration with Jenkins

OK. We have a check that can fail our build if there're some errors and we have the report which can be used for results analysis. So, it's time to bind all that stuff into Jenkins. To our luck there're several plugins for that. For now I'm mostly interested with the Checkstyle Plugin and PMD Plugin. Once we enable it we have additional post-build step action for reports collection. For this purpose we should:

  1. Open Jenkins project we want to add report processing actions to
  2. Navigate to the Configure page
  3. Scroll down to the Post-build actions
  4. Click on Add post-build step button and select Publish Check Style analysis results option
The configuration section looks like:
where you can define the mask to look for the reports by and various options which help Jenkins identify whether build is treated as stable or require attention based on the number and warning types.

Once we configure that we can move further and cover other modules.

C# modules static testing

Installation

StyleCop can be downloaded from the official site. Another option is using NuGet to get the binaries from the storage. There're dedicated packages like:

and many others. In any case what we are mostly interested in is: Once we have it all we can move further.

Configuration

StyleCop has it's own build task which can be used in MSBuild. In order to use them we should update our MSBuild script with the following entry:

 <Import Project="$(ProgramFiles)\MSBuild\StyleCop\v4.7\StyleCop.targets" /> 
That's the location where StyleCop is installed by default. If we use NuGet packages we should reference to the location where the files are retrieved to.

Once we include this file we can use the StyleCop task. however, we should specify parameters which should be used with it. For this purpose we should update MSBuild script with the entries like:

 <PropertyGroup>
  <StyleCopForceFullAnalysis>true</StyleCopForceFullAnalysis>
  <StyleCopCacheResults>false</StyleCopCacheResults>
  <StyleCopOutputFile>$(SolutionDir)\StyleCop.xml</StyleCopOutputFile>
  <StyleCopTreatErrorsAsWarnings>false</StyleCopTreatErrorsAsWarnings>
 </PropertyGroup>
 <ItemGroup>
  <StyleCopFiles Include="$(SolutionDir)\\**\*.cs" />
 </ItemGroup>
The key part of the configuration is highlighted. Here we define the list of files to check. We can use comma-separated list of file names or we can use mask as I did in this example. In any case we should specify the file set corresponding to the list of files we should check.

Additionally, I'd like to mention the StyleCopTreatErrorsAsWarnings flag setting. If we set is as true all the errors the StyleCop finds will be displayed as build warnings. Otherwise, StyleCop will output errors and they will fail the entire build. I'm looking for the second option so I set the StyleCopTreatErrorsAsWarnings flag to false.

And final update is the dependency. E.g. I want to perform StyleCop check every time I make a build. So, if I have related task (e.g. named Build) I can add the dependency like:

 <Target Name="Build"  DependsOnTargets="Clean;StyleCop">

Usage

Once it's done we can start using MSBuild script as usual:

C:\Windows\Microsoft.NET\Framework\v4.0.30319\MSBuild.exe BuildScript\Build.msbuild /t:Build
In case some style violations it will produce the output like:
 D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(8,1): error : SA1600 : CSharp.Documentation : The class must have a documentation header. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]
  D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(1,1): error : SA1633 : CSharp.Documentation : The file has no header, the header Xml is invalid, or the header is not located at the top of the file. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]
  D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(1,1): error : SA1200 : CSharp.Ordering : All using directives must be placed inside of the namespace. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]
  D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(2,1): error : SA1200 : CSharp.Ordering : All using directives must be placed inside of the namespace. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]
  D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(3,1): error : SA1200 : CSharp.Ordering : All using directives must be placed inside of the namespace. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]
  D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\Class1.cs(4,1): error : SA1200 : CSharp.Ordering : All using directives must be placed inside of the namespace. [D:\Work\Jenkins\.hudson\jobs\Sirius_CSharp_Client_Core\workspace\Sirius.Client.Core\BuildScript\Build.msbuild]

    Warnings: 0
    Errors: 6
Also, it will produce the report which path is specified with StyleCopOutputFile option in the MSBuild script. Here is the sample content:
<StyleCopViolations>
  <Violation 
    Section="Root" 
    LineNumber="1" 
    Source="D:\Work\SiriusDev\Sirius\SiriusCSharp.Client\SiriusClient\Properties\AssemblyInfo.cs" 
    RuleNamespace="StyleCop.CSharp.DocumentationRules" 
    Rule="FileMustHaveHeader" 
    RuleId="SA1633">
        The file has no header, the header Xml is invalid, or the header is not located at the top of the file.
    </Violation>
</StyleCopViolations>

Integration with Jenkins

The Jenkins execution won't change as we add StyleCop task implicitly. The only thing we should add for Jenkins is the report collection. For this purpose there's Static Analysis Collector Plugin. It's actually designed not only for StyleCop but also for many other utilities including StyleCheck and PMD. However, now it's applicable the most. So, once we installed it we should:

  1. Open Jenkins project we want to add report processing actions to
  2. Navigate to the Configure page
  3. Scroll down to the Post-build actions
  4. Click on Add post-build step button and select Report Violations option
There would be a big table where we should enter the parameters for each static checking engine we want the report to publish. Here we should fill the fields near stylecop section. In my example I should fill something like: **/StyleCop.xml. Also I should fill the Source Path Pattern field. For C# projects the pattern usually is: **/*.cs.

That's it. Now we can run our builds with Jenkins.

Ruby modules static testing

Installation

Rubocop is available as gem package so it can be installed using the following command:

gem install rubocop
After that it can be accessible from command line.

Configuration

All we have to update is the Rakefile with the new task like:

task :rubocop do |t|
  sh 'call rubocop -c .rubocop.yml -e lib'
end
Here we call Rubocop with 2 parameters:
  • The configuration file where we define which rules are to be checked
  • The folder to check sources in
The configuration file contains the list of rules to be checked as well as the flags saying whether the check should be enabled or not. Here is the sample fragment of the rubocop.yml file:
Encoding:
  Enabled: false

LineLength:
  Enabled: true
  Max: 79

Tab:
  Enabled: true
We can turn on/off any check we see appropriate.

Usage

Once the configuration is done we can trigger the style checking with the following command:

rake rubocop
All the violations will be written into the console output. We can customize it actually, but for now the main thing is that when some violations occur the build will fail.

Summary

The static testing is more than simple code style verifications and there may be more engines to add. However the major step is done. Now we're releasing not only working code but also well-formatted and well-designed code which can be used further. That's the main reason why I introduced the code style verifications into the Sirius build process.

And one more thing. Sometimes that can be annoying fixing the style violations as in most cases it's just decoration and doesn't add new value however it's a good way to bring the team a habit to write the code in the same style and with all necessary requisites. So, the good style should become a good habit.

1 comment:

  1. This solution is more complicated and covers many different technologies. For regular development StyleCop is quite enough.

    ReplyDelete