Pages

Saturday, March 08, 2014

Reviewing CDK patches in the Maven era

Three weeks ago the CDK project migrated from Ant to Maven as the primary build tool. That means that my workflow for making and, importantly, reviewing patches is completely turned upside down. Well, that happens.

My patch reviewing workflow looks like:
  1. run the test suite and capture the number of JUnit Errors and Fails
  2. apply the patch and check if things still compile
  3. run the test suite and capture the number of JUnit Errors and Fails
  4. compare the number of Errors and Fails before and after
  5. check if JavaDoc is in order
  6. check if there is new unit testsing where appropriate
  7. check for new PMD issues
In there issues I always had CDK Nightly as backup, and this is now replaced by Jenkins; e.g. check this instance at the EBI. This workflow now translate to something like this (the extraction of the results was suggested by John):
  1. mvn clean compile test -Dmaven.test.failure.ignore=true
  2. cat */*/target/surefire-reports/* | grep "Tests run" | sed -e "s/, Time elapsed.* /\|/" | sort -t'|' -k2 > prepatch.txt
  3. git am / git cherry-pick
  4. repeat step 1 and 2, and safe as postpatch.txt
  5. diff -u prepatch.txt postpatch.txt
  6. repeat step 1-5, if needed.
    And if all is good, then the diff should show no new fails and possibly even less. During a set of patches, things may be temporary failing, such as in this case:

    diff -u prepatch.txt postpatch.txt 
    --- prepatch.txt        2014-03-08 11:41:13.520240111 +0100
    +++ postpatch.txt       2014-03-08 12:59:21.022609259 +0100
    @@ -3,6 +3,14 @@
     Tests run: 22, Failures: 1, Errors: 0, Skipped: 0|org.openscience.cdk.atomtype.ReactionStructuresTest
     Tests run: 1, Failures: 1, Errors: 0, Skipped: 0|org.openscience.cdk.CDKTest
     Tests run: 10, Failures: 1, Errors: 0, Skipped: 0|org.openscience.cdk.formula.rules.IsotopePatternRuleTest
    +Tests run: 15, Failures: 0, Errors: 10, Skipped: 0|org.openscience.cdk.graph.CyclesTest
    +Tests run: 14, Failures: 0, Errors: 14, Skipped: 0|org.openscience.cdk.graph.EdgeShortCyclesTest
    +Tests run: 12, Failures: 0, Errors: 12, Skipped: 0|org.openscience.cdk.graph.EssentialCyclesTest
    +Tests run: 31, Failures: 0, Errors: 18, Skipped: 0|org.openscience.cdk.graph.InitialCyclesTest
    +Tests run: 14, Failures: 0, Errors: 12, Skipped: 0|org.openscience.cdk.graph.MinimumCycleBasisTest
    +Tests run: 14, Failures: 0, Errors: 12, Skipped: 0|org.openscience.cdk.graph.RelevantCyclesTest
    +Tests run: 13, Failures: 0, Errors: 11, Skipped: 0|org.openscience.cdk.graph.TripletShortCyclesTest
    +Tests run: 14, Failures: 0, Errors: 14, Skipped: 0|org.openscience.cdk.graph.VertexShortCyclesTest
     Tests run: 2, Failures: 2, Errors: 0, Skipped: 0|org.openscience.cdk.io.cml.QSARCMLRoundTripTest
     Tests run: 14, Failures: 5, Errors: 0, Skipped: 0|org.openscience.cdk.modeling.builder3d.ForceFieldConfiguratorTest
     Tests run: 15, Failures: 1, Errors: 0, Skipped: 0|org.openscience.cdk.qsar.descriptors.atomic.AtomDegreeDescriptorTest

    Oh, and intermediate compiles I can do without running the tests with:

        mvn compile -DskipTests