Merge pull request #102 from adangel:pmd-long-cli-options

Use long cli options for PMD >= 6.41.0 #102

* pr-102:
  Fix semantic version comparison
  Use long cli options for PMD >= 6.41.0

Author: adangel

Manifest.txt

@@ -44,6 +44,7 @@ lib/pmdtester/project.rb
 lib/pmdtester/report_diff.rb
 lib/pmdtester/resource_locator.rb
 lib/pmdtester/runner.rb
+lib/pmdtester/semver.rb
 pmdtester.gemspec
 resources/_includes/diff_pill_row.html
 resources/css/bootstrap.min.css

lib/pmdtester.rb

@@ -15,6 +15,7 @@
 require_relative 'pmdtester/report_diff'
 require_relative 'pmdtester/resource_locator'
 require_relative 'pmdtester/runner'
+require_relative 'pmdtester/semver'
 
 require_relative 'pmdtester/builders/simple_progress_logger'
 require_relative 'pmdtester/builders/project_builder'

lib/pmdtester/builders/pmd_report_builder.rb

@@ -97,11 +97,12 @@ def get_last_commit_message
     def generate_pmd_report(project)
       error_recovery_options = @error_recovery ? 'PMD_JAVA_OPTS="-Dpmd.error_recovery -ea" ' : ''
       run_path = "#{saved_distro_path(@pmd_branch_details.branch_last_sha)}/bin/run.sh"
+      fail_on_violation = should_use_long_cli_options ? '--fail-on-violation false' : '-failOnViolation false'
       pmd_cmd = "#{error_recovery_options}" \
                 "#{run_path} pmd -d #{project.local_source_path} -f xml " \
                 "-R #{project.get_config_path(@pmd_branch_name)} " \
                 "-r #{project.get_pmd_report_path(@pmd_branch_name)} " \
-                "-failOnViolation false -t #{@threads} " \
+                "#{fail_on_violation} -t #{@threads} " \
                 "#{project.auxclasspath}"
       start_time = Time.now
       if File.exist?(project.get_pmd_report_path(@pmd_branch_name))
@@ -194,5 +195,10 @@ def saved_distro_path(build_sha)
     def wd_has_dirty_git_changes
       !Cmd.execute('git status --porcelain').empty?
     end
+
+    def should_use_long_cli_options
+      logger.debug "PMD Version: #{@pmd_version}"
+      Semver.compare(@pmd_version, '6.41.0') >= 0
+    end
   end
 end

lib/pmdtester/semver.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module PmdTester
+  # Utility to deal with semantic versions
+  class Semver
+    def self.compare(version_a, version_b)
+      PmdTester.logger.debug "Comparing #{version_a} <=> #{version_b}"
+      m = /(\d+)\.(\d+)\.(\d+)(.*)/.match(version_a)
+      a_major = m[1].to_i
+      a_minor = m[2].to_i
+      a_patch = m[3].to_i
+      a_snapshot = m[4]
+      PmdTester.logger.debug "  a_major: #{a_major} a_minor: #{a_minor} a_patch: #{a_patch} a_snapshot: #{a_snapshot}"
+
+      m = /(\d+)\.(\d+)\.(\d+)(.*)/.match(version_b)
+      b_major = m[1].to_i
+      b_minor = m[2].to_i
+      b_patch = m[3].to_i
+      b_snapshot = m[4]
+      PmdTester.logger.debug "  b_major: #{b_major} b_minor: #{b_minor} b_patch: #{b_patch} b_snapshot: #{b_snapshot}"
+
+      return a_major <=> b_major if a_major != b_major
+      return a_minor <=> b_minor if a_minor != b_minor
+      return a_patch <=> b_patch if a_patch != b_patch
+
+      compare_snapshots(a_snapshot, b_snapshot)
+    end
+
+    private_class_method def self.compare_snapshots(a_snapshot, b_snapshot)
+      return -1 if a_snapshot == '-SNAPSHOT' && b_snapshot == ''
+      return 1 if a_snapshot == '' && b_snapshot == '-SNAPSHOT'
+
+      a_snapshot <=> b_snapshot
+    end
+  end
+end

pmdtester.gemspec

@@ -11,12 +11,12 @@ Gem::Specification.new do |s|
   s.metadata = { "bug_tracker_uri" => "https://github.com/pmd/pmd-regression-tester/issues", "homepage_uri" => "https://pmd.github.io", "source_code_uri" => "https://github.com/pmd/pmd-regression-tester" } if s.respond_to? :metadata=
   s.require_paths = ["lib".freeze]
   s.authors = ["Andreas Dangel".freeze, "Binguo Bao".freeze, "Cl\u00E9ment Fournier".freeze]
-  s.date = "2021-06-20"
+  s.date = "2021-12-17"
   s.description = "A regression testing tool ensure that new problems and unexpected behaviors will not be introduced to PMD project after fixing an issue , and new rules can work as expected.".freeze
   s.email = ["[email protected]".freeze, "[email protected]".freeze, "[email protected]".freeze]
   s.executables = ["pmdtester".freeze]
   s.extra_rdoc_files = ["History.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze]
-  s.files = [".ci/build.sh".freeze, ".ci/inc/fetch_ci_scripts.bash".freeze, ".ci/manual-integration-tests.sh".freeze, ".github/workflows/build.yml".freeze, ".github/workflows/manual-integration-tests.yml".freeze, ".gitignore".freeze, ".hoerc".freeze, ".rubocop.yml".freeze, ".rubocop_todo.yml".freeze, ".ruby-version".freeze, "Gemfile".freeze, "History.md".freeze, "LICENSE".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "Rakefile".freeze, "bin/pmdtester".freeze, "config/all-java.xml".freeze, "config/design.xml".freeze, "config/project-list.xml".freeze, "config/projectlist_1_0_0.xsd".freeze, "config/projectlist_1_1_0.xsd".freeze, "config/projectlist_1_2_0.xsd".freeze, "lib/pmdtester.rb".freeze, "lib/pmdtester/builders/liquid_renderer.rb".freeze, "lib/pmdtester/builders/pmd_report_builder.rb".freeze, "lib/pmdtester/builders/project_builder.rb".freeze, "lib/pmdtester/builders/project_hasher.rb".freeze, "lib/pmdtester/builders/rule_set_builder.rb".freeze, "lib/pmdtester/builders/simple_progress_logger.rb".freeze, "lib/pmdtester/builders/summary_report_builder.rb".freeze, "lib/pmdtester/cmd.rb".freeze, "lib/pmdtester/collection_by_file.rb".freeze, "lib/pmdtester/parsers/options.rb".freeze, "lib/pmdtester/parsers/pmd_report_document.rb".freeze, "lib/pmdtester/parsers/projects_parser.rb".freeze, "lib/pmdtester/pmd_branch_detail.rb".freeze, "lib/pmdtester/pmd_configerror.rb".freeze, "lib/pmdtester/pmd_error.rb".freeze, "lib/pmdtester/pmd_report_detail.rb".freeze, "lib/pmdtester/pmd_tester_utils.rb".freeze, "lib/pmdtester/pmd_violation.rb".freeze, "lib/pmdtester/project.rb".freeze, "lib/pmdtester/report_diff.rb".freeze, "lib/pmdtester/resource_locator.rb".freeze, "lib/pmdtester/runner.rb".freeze, "pmdtester.gemspec".freeze, "resources/_includes/diff_pill_row.html".freeze, "resources/css/bootstrap.min.css".freeze, "resources/css/datatables.min.css".freeze, "resources/css/pmd-tester.css".freeze, "resources/js/bootstrap.min.js".freeze, "resources/js/code-snippets.js".freeze, "resources/js/datatables.min.js".freeze, "resources/js/jquery-3.2.1.slim.min.js".freeze, "resources/js/jquery.min.js".freeze, "resources/js/popper.min.js".freeze, "resources/js/project-report.js".freeze, "resources/project_diff_report.html".freeze, "resources/project_index.html".freeze, "resources/project_pmd_report.html".freeze]
+  s.files = [".ci/build.sh".freeze, ".ci/inc/fetch_ci_scripts.bash".freeze, ".ci/manual-integration-tests.sh".freeze, ".github/workflows/build.yml".freeze, ".github/workflows/manual-integration-tests.yml".freeze, ".gitignore".freeze, ".hoerc".freeze, ".rubocop.yml".freeze, ".rubocop_todo.yml".freeze, ".ruby-version".freeze, "Gemfile".freeze, "History.md".freeze, "LICENSE".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "Rakefile".freeze, "bin/pmdtester".freeze, "config/all-java.xml".freeze, "config/design.xml".freeze, "config/project-list.xml".freeze, "config/projectlist_1_0_0.xsd".freeze, "config/projectlist_1_1_0.xsd".freeze, "config/projectlist_1_2_0.xsd".freeze, "lib/pmdtester.rb".freeze, "lib/pmdtester/builders/liquid_renderer.rb".freeze, "lib/pmdtester/builders/pmd_report_builder.rb".freeze, "lib/pmdtester/builders/project_builder.rb".freeze, "lib/pmdtester/builders/project_hasher.rb".freeze, "lib/pmdtester/builders/rule_set_builder.rb".freeze, "lib/pmdtester/builders/simple_progress_logger.rb".freeze, "lib/pmdtester/builders/summary_report_builder.rb".freeze, "lib/pmdtester/cmd.rb".freeze, "lib/pmdtester/collection_by_file.rb".freeze, "lib/pmdtester/parsers/options.rb".freeze, "lib/pmdtester/parsers/pmd_report_document.rb".freeze, "lib/pmdtester/parsers/projects_parser.rb".freeze, "lib/pmdtester/pmd_branch_detail.rb".freeze, "lib/pmdtester/pmd_configerror.rb".freeze, "lib/pmdtester/pmd_error.rb".freeze, "lib/pmdtester/pmd_report_detail.rb".freeze, "lib/pmdtester/pmd_tester_utils.rb".freeze, "lib/pmdtester/pmd_violation.rb".freeze, "lib/pmdtester/project.rb".freeze, "lib/pmdtester/report_diff.rb".freeze, "lib/pmdtester/resource_locator.rb".freeze, "lib/pmdtester/runner.rb".freeze, "lib/pmdtester/semver.rb".freeze, "pmdtester.gemspec".freeze, "resources/_includes/diff_pill_row.html".freeze, "resources/css/bootstrap.min.css".freeze, "resources/css/datatables.min.css".freeze, "resources/css/pmd-tester.css".freeze, "resources/js/bootstrap.min.js".freeze, "resources/js/code-snippets.js".freeze, "resources/js/datatables.min.js".freeze, "resources/js/jquery-3.2.1.slim.min.js".freeze, "resources/js/jquery.min.js".freeze, "resources/js/popper.min.js".freeze, "resources/js/project-report.js".freeze, "resources/project_diff_report.html".freeze, "resources/project_index.html".freeze, "resources/project_pmd_report.html".freeze]
   s.homepage = "https://pmd.github.io".freeze
   s.licenses = ["BSD-2-Clause".freeze]
   s.rdoc_options = ["--main".freeze, "README.rdoc".freeze]
@@ -42,7 +42,7 @@ Gem::Specification.new do |s|
     s.add_development_dependency(%q<rubocop>.freeze, ["~> 0.81"])
     s.add_development_dependency(%q<test-unit>.freeze, ["~> 3.2"])
     s.add_development_dependency(%q<rdoc>.freeze, [">= 4.0", "< 7"])
-    s.add_development_dependency(%q<hoe>.freeze, ["~> 3.22"])
+    s.add_development_dependency(%q<hoe>.freeze, ["~> 3.23"])
   else
     s.add_dependency(%q<nokogiri>.freeze, [">= 1.11.0.rc4"])
     s.add_dependency(%q<slop>.freeze, ["~> 4.6"])
@@ -57,7 +57,7 @@ Gem::Specification.new do |s|
     s.add_dependency(%q<rubocop>.freeze, ["~> 0.81"])
     s.add_dependency(%q<test-unit>.freeze, ["~> 3.2"])
     s.add_dependency(%q<rdoc>.freeze, [">= 4.0", "< 7"])
-    s.add_dependency(%q<hoe>.freeze, ["~> 3.22"])
+    s.add_dependency(%q<hoe>.freeze, ["~> 3.23"])
   end
 end
 

test/test_pmd_report_builder.rb

@@ -127,22 +127,43 @@ def test_build_error_recovery
       .build
   end
 
+  def test_build_long_cli_options
+    @pmd_version = '6.41.0'
+    project_list = 'test/resources/pmd_report_builder/project-list.xml'
+    projects = PmdTester::ProjectsParser.new.parse(project_list)
+    assert_equal(1, projects.size)
+    argv = %w[-r target/repositories/pmd -b master -p pmd_releases/6.1.0
+              -c config/design.xml --debug --error-recovery -l]
+    argv.push project_list
+    options = PmdTester::Options.new(argv)
+
+    projects[0].auxclasspath = '-auxclasspath extra:dirs'
+    record_expectations('sha1abc', 'sha1abc', true)
+    record_expectations_after_build
+    record_expectations_project_build('sha1abc', true, true)
+
+    PmdTester::PmdReportBuilder
+      .new(projects, options, options.base_config, options.base_branch)
+      .build
+  end
+
   private
 
-  def record_expectations_project_build(sha1, error = false)
+  def record_expectations_project_build(sha1, error = false, long_cli_options = false)
     PmdTester::ProjectBuilder.any_instance.stubs(:clone_projects).once
     PmdTester::ProjectBuilder.any_instance.stubs(:build_projects).once
     PmdTester::SimpleProgressLogger.any_instance.stubs(:start).once
     PmdTester::SimpleProgressLogger.any_instance.stubs(:stop).once
     error_prefix = error ? 'PMD_JAVA_OPTS="-Dpmd.error_recovery -ea" ' : ''
     distro_path = "#{Dir.getwd}/target/pmd-bin-#{@pmd_version}-master-#{sha1}"
+    fail_on_violation = long_cli_options ? '--fail-on-violation false' : '-failOnViolation false'
     PmdTester::Cmd.stubs(:execute)
                   .with("#{error_prefix}" \
                         "#{distro_path}/bin/run.sh " \
                         'pmd -d target/repositories/checkstyle -f xml ' \
                         '-R target/reports/master/checkstyle/config.xml ' \
                         '-r target/reports/master/checkstyle/pmd_report.xml ' \
-                        '-failOnViolation false -t 1 ' \
+                        "#{fail_on_violation} -t 1 " \
                         '-auxclasspath extra:dirs').once
     PmdTester::PmdReportDetail.any_instance.stubs(:save).once
   end

test/test_semver.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+# Unit test class for PmdTester::Semver
+class TestSemver < Test::Unit::TestCase
+  def test_compare_releases
+    assert_equal(0, PmdTester::Semver.compare('1.0.0', '1.0.0'))
+    assert_equal(-1, PmdTester::Semver.compare('1.0.0', '1.0.1'))
+    assert_equal(-1, PmdTester::Semver.compare('1.0.0', '1.1.0'))
+    assert_equal(-1, PmdTester::Semver.compare('1.0.0', '2.0.0'))
+    assert_equal(1, PmdTester::Semver.compare('1.1.0', '1.0.0'))
+    assert_equal(1, PmdTester::Semver.compare('2.0.0', '1.0.0'))
+
+    assert_equal(1, PmdTester::Semver.compare('6.42.0', '6.41.0'))
+    assert_equal(1, PmdTester::Semver.compare('7.0.0', '6.41.0'))
+    assert_equal(0, PmdTester::Semver.compare('6.41.0', '6.41.0'))
+    assert_equal(-1, PmdTester::Semver.compare('6.40.0', '6.41.0'))
+  end
+
+  def test_compare_snapshots
+    assert_equal(1, PmdTester::Semver.compare('7.0.0-SNAPSHOT', '6.41.0'))
+    assert_equal(-1, PmdTester::Semver.compare('6.41.0-SNAPSHOT', '6.41.0'))
+    assert_equal(0, PmdTester::Semver.compare('6.41.0-SNAPSHOT', '6.41.0-SNAPSHOT'))
+  end
+end