Merge branch 'master' into better-html-reports

oowekyala · oowekyala · commit b2d6a9a86460 · 2020-11-23T12:37:54.000+01:00
diff --git a/History.md b/History.md
@@ -15,6 +15,8 @@ Typeresolution is now supported by two new tags in the project-list.xml file:
 *   [#69](https://github.com/pmd/pmd-regression-tester/pull/69): Detect single rules with auto-gen-config
 *   [#70](https://github.com/pmd/pmd-regression-tester/pull/70): Add link to PR on github in HTML report
 *   [#74](https://github.com/pmd/pmd-regression-tester/pull/74): Merge violations that have just changed messages
+*   [#75](https://github.com/pmd/pmd-regression-tester/pull/75): Add new option "--error-recovery"
+*   [#76](https://github.com/pmd/pmd-regression-tester/pull/76): Speedup XML parsing
 
 ## External Contributions
 
diff --git a/README.rdoc b/README.rdoc
@@ -39,6 +39,7 @@ on a list of standard projects(Spring Framework, Hibernate, Solr, etc.)
     -a, --auto-gen-config  whether to generate configurations automatically based on branch differences,this option only works in online and local mode
     --keep-reports         whether to keep old reports and skip running PMD again if possible
     -d, --debug            whether change log level to DEBUG to see more information
+    --error-recovery       enable error recovery mode when executing PMD. Might help to analyze errors.
     -v, --version
     -h, --help
 
diff --git a/lib/pmdtester/builders/pmd_report_builder.rb b/lib/pmdtester/builders/pmd_report_builder.rb
@@ -9,16 +9,17 @@ module PmdTester
   class PmdReportBuilder
     include PmdTester
 
-    def initialize(branch_config, projects, local_git_repo, pmd_branch_name, threads = 1)
-      @branch_config = branch_config
+    def initialize(projects, options, branch_config, branch_name)
       @projects = projects
-      @local_git_repo = local_git_repo
-      @pmd_branch_name = pmd_branch_name
-      @threads = threads
+      @local_git_repo = options.local_git_repo
+      @threads = options.threads
+      @error_recovery = options.error_recovery
+      @branch_config = branch_config
+      @pmd_branch_name = branch_name
       @pwd = Dir.getwd
 
-      @pmd_branch_details = PmdBranchDetail.new(pmd_branch_name)
-      @project_builder = ProjectBuilder.new(projects)
+      @pmd_branch_details = PmdBranchDetail.new(@pmd_branch_name)
+      @project_builder = ProjectBuilder.new(@projects)
     end
 
     def get_pmd_binary_file
@@ -84,8 +85,10 @@ def get_last_commit_message
     end
 
     def generate_pmd_report(project)
+      error_recovery_options = @error_recovery ? 'PMD_JAVA_OPTS="-Dpmd.error_recovery -ea" ' : ''
       run_path = "target/pmd-bin-#{@pmd_version}/bin/run.sh"
-      pmd_cmd = "#{run_path} pmd -d #{project.local_source_path} -f xml " \
+      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} " \
diff --git a/lib/pmdtester/builders/project_builder.rb b/lib/pmdtester/builders/project_builder.rb
@@ -89,7 +89,7 @@ def run_as_script(path, command)
     def execute_reset_cmd(type, tag)
       case type
       when 'git'
-        reset_cmd = "git reset --hard #{tag}"
+        reset_cmd = "git checkout #{tag}; git reset --hard #{tag}"
       when 'hg'
         reset_cmd = "hg up #{tag}"
       end
diff --git a/lib/pmdtester/parsers/options.rb b/lib/pmdtester/parsers/options.rb
@@ -31,6 +31,7 @@ class Options
     attr_reader :debug_flag
     attr_accessor :filter_set
     attr_reader :keep_reports
+    attr_reader :error_recovery
 
     def initialize(argv)
       options = parse(argv)
@@ -48,6 +49,7 @@ def initialize(argv)
       @debug_flag = options[:d]
       @filter_set = nil
       @keep_reports = options.keep_reports?
+      @error_recovery = options.error_recovery?
 
       # if the 'config' option is selected then `config` overrides `base_config` and `patch_config`
       @base_config = @config if !@config.nil? && @mode == 'local'
@@ -94,6 +96,8 @@ def parse(argv)
                'whether to keep old reports and skip running PMD again if possible'
         o.bool '-d', '--debug',
                'whether change log level to DEBUG to see more information'
+        o.bool '--error-recovery',
+               'enable error recovery mode when executing PMD. Might help to analyze errors.'
         o.on '-v', '--version' do
           puts VERSION
           exit
diff --git a/lib/pmdtester/parsers/pmd_report_document.rb b/lib/pmdtester/parsers/pmd_report_document.rb
@@ -62,11 +62,9 @@ def end_element(name)
         @violations.add_all(@current_filename, @current_violations)
         @current_filename = nil
       when 'violation'
-        v = @current_violation
-        if match_filter_set?(v)
-          v.message = finish_text!
-          @current_violations.push(v)
-          @infos_by_rules[v.rule_name] = RuleInfo.new(v.rule_name, v.info_url) unless @infos_by_rules.key?(v.rule_name)
+        if match_filter_set?(@current_violation)
+          @current_violation.message = finish_text!
+          @current_violations.push(@current_violation)
         end
         @current_violation = nil
       when 'error'
@@ -87,15 +85,12 @@ def finish_text!
     def match_filter_set?(violation)
       return true if @filter_set.nil?
 
-      @filter_set.each do |filter_rule_ref|
-        ruleset_attr = violation.ruleset_file
-        rule = violation.rule_name
-        rule_ref = "#{ruleset_attr}/#{rule}"
-        return true if filter_rule_ref.eql?(ruleset_attr)
-        return true if filter_rule_ref.eql?(rule_ref)
-      end
+      ruleset_attr = violation.attrs['ruleset'].delete(' ').downcase + '.xml'
+      return true if @filter_set.include?(ruleset_attr)
+
+      rule_ref = "#{ruleset_attr}/#{violation.rule_name}"
 
-      false
+      @filter_set.include?(rule_ref)
     end
   end
 end
diff --git a/lib/pmdtester/runner.rb b/lib/pmdtester/runner.rb
@@ -142,9 +142,7 @@ def summarize_diffs
     private
 
     def make_branch_details(config:, branch:)
-      PmdReportBuilder
-        .new(config, @projects, @options.local_git_repo, branch, @options.threads)
-        .build
+      PmdReportBuilder.new(@projects, @options, config, branch).build
     end
   end
 end
diff --git a/pmdtester.gemspec b/pmdtester.gemspec
@@ -11,7 +11,7 @@ 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]
-  s.date = "2020-11-19"
+  s.date = "2020-11-23"
   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 = ["andreas.dangel@pmd-code.org".freeze, "djydewang@gmail.com".freeze]
   s.executables = ["pmdtester".freeze]
diff --git a/test/integration_test_pmd_report_builder.rb b/test/integration_test_pmd_report_builder.rb
@@ -10,12 +10,16 @@ def setup
   end
 
   def test_build
-    logger.level = Logger::INFO
-    projects = ProjectsParser.new.parse('test/resources/project-test.xml')
-    builder = PmdReportBuilder.new('config/design.xml', projects,
-                                   'target/repositories/pmd', 'pmd_releases/6.7.0')
+    argv = %w[-r target/repositories/pmd -b master -p origin/pmd/7.0.x
+              -c test/resources/pmd7-config.xml -l test/resources/project-test.xml
+              --error-recovery --debug]
+    options = PmdTester::Options.new(argv)
+    projects = ProjectsParser.new.parse(options.project_list)
+
+    builder = PmdReportBuilder.new(projects, options, options.config, options.patch_branch)
     builder.build
 
     assert_equal(0, $CHILD_STATUS.exitstatus)
+    assert_path_exist('target/reports/origin_pmd_7.0.x/checkstyle/pmd_report.xml')
   end
 end
diff --git a/test/resources/pmd7-config.xml b/test/resources/pmd7-config.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+
+<ruleset name="Java Rules for PMD7 integration test" 
+    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+    <description>Java Rules for PMD7 integration test</description>
+
+    <rule ref="category/java/bestpractices.xml/MissingOverride"/>
+    <rule ref="category/java/bestpractices.xml/UnusedAssignment"/>
+</ruleset>
diff --git a/test/resources/project-test.xml b/test/resources/project-test.xml
@@ -9,8 +9,8 @@ xsi:noNamespaceSchemaLocation="projectlist_1_1_0.xsd">
     <type>git</type>
     <connection>https://github.com/checkstyle/checkstyle</connection>
     <tag>checkstyle-8.0</tag>
-    <exclude-pattern>.*/src/test/resources-noncompilable/.*</exclude-pattern>
-    <exclude-pattern>.*/src/test/resources/.*</exclude-pattern>
+    <exclude-pattern>.*/target/test-classes/com/puppycrawl/tools/checkstyle/.*</exclude-pattern>
+    <exclude-pattern>.*/target/generated-sources/.*</exclude-pattern>
 
     <build-command><![CDATA[#!/usr/bin/env bash
 if test -e classpath.txt; then
@@ -23,6 +23,7 @@ mvn dependency:build-classpath -DincludeScope=test -Dmdep.outputFile=classpath.t
     <auxclasspath-command>echo -n "$(pwd)/target/classes:$(pwd)/target/test-classes:"; cat classpath.txt</auxclasspath-command>
   </project>
 
+  <!-- this project configuration is here, so that PMD's repo is cloned into target/repositories for integration tests -->
   <project>
     <name>pmd</name>
     <type>git</type>
diff --git a/test/test_options.rb b/test/test_options.rb
@@ -38,6 +38,13 @@ def test_default_value
     assert_equal(Options::DEFAULT_CONFIG_PATH, opts.base_config)
     assert_equal(Options::DEFAULT_CONFIG_PATH, opts.patch_config)
     assert_equal(Options::DEFAULT_LIST_PATH, opts.project_list)
+    assert_false(opts.error_recovery)
+  end
+
+  def test_enable_error_recovery
+    command_line = %w[-r /path/to/repo -b pmd_releases/6.2.0 -p master --error-recovery]
+    opts = Options.new(command_line)
+    assert_true(opts.error_recovery)
   end
 
   def test_single_mode
diff --git a/test/test_pmd_report_builder.rb b/test/test_pmd_report_builder.rb
@@ -14,7 +14,7 @@ def test_build_skip
     record_expecations_after_build
 
     PmdTester::PmdReportBuilder
-      .new(options.base_config, projects, options.local_git_repo, options.base_branch)
+      .new(projects, options, options.base_config, options.base_branch)
       .build
   end
 
@@ -33,7 +33,7 @@ def test_build_normal
     record_expecations_after_build
 
     PmdTester::PmdReportBuilder
-      .new(options.base_config, projects, options.local_git_repo, options.base_branch)
+      .new(projects, options, options.base_config, options.base_branch)
       .build
   end
 
@@ -52,24 +52,45 @@ def test_build_with_projects
     record_expectations_project_build
 
     PmdTester::PmdReportBuilder
-      .new(options.base_config, projects, options.local_git_repo, options.base_branch)
+      .new(projects, options, options.base_config, options.base_branch)
       .build
 
     expected = File.read('test/resources/pmd_report_builder/expected-config.xml')
     actual = File.read('target/reports/master/checkstyle/config.xml')
     assert_equal(expected, actual)
   end
 
+  def test_build_error_recovery
+    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_expecations_after_build
+    record_expectations_project_build(true)
+
+    PmdTester::PmdReportBuilder
+      .new(projects, options, options.base_config, options.base_branch)
+      .build
+  end
+
   private
 
-  def record_expectations_project_build
+  def record_expectations_project_build(error = 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
     File.stubs(:exist?).with('target/reports/master/checkstyle/pmd_report.xml').returns(false).once
+    error_prefix = error ? 'PMD_JAVA_OPTS="-Dpmd.error_recovery -ea" ' : ''
     PmdTester::Cmd.stubs(:execute)
-                  .with('target/pmd-bin-6.10.0-SNAPSHOT/bin/run.sh ' \
+                  .with("#{error_prefix}" \
+                        'target/pmd-bin-6.10.0-SNAPSHOT/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 ' \
diff --git a/test/test_project_builder.rb b/test/test_project_builder.rb
@@ -12,9 +12,9 @@ def setup
 
   def test_clone
     expect_clone('checkstyle', 'git', 'https://github.com/checkstyle/checkstyle',
-                 'git reset --hard master')
+                 'git checkout master; git reset --hard master')
     expect_clone('spring-framework', 'git', 'https://github.com/spring-projects/spring-framework',
-                 'git reset --hard v5.0.6.RELEASE')
+                 'git checkout v5.0.6.RELEASE; git reset --hard v5.0.6.RELEASE')
 
     project_builder = PmdTester::ProjectBuilder.new(@projects)
     project_builder.clone_projects
diff --git a/test/test_runner.rb b/test/test_runner.rb
@@ -33,7 +33,9 @@ def test_single_mode
   def test_single_mode_multithreading
     report_builder_mock = mock
     PmdReportBuilder.stubs(:new)
-                    .with(anything, anything, anything, anything, 4)
+                    .with do |_, options, _, _|
+                      options.threads == 4
+                    end
                     .returns(report_builder_mock)
                     .once
     report_builder_mock.stubs(:build).returns(PmdBranchDetail.new('test_branch')).once
@@ -59,15 +61,17 @@ def test_local_mode
   def test_local_mode_multithreading
     report_builder_mock = mock
     PmdReportBuilder.stubs(:new)
-                    .with(anything, anything, anything, anything, 4)
+                    .with do |_, options, _, _|
+                      options.threads == 5
+                    end
                     .returns(report_builder_mock)
                     .twice
     report_builder_mock.stubs(:build).returns(PmdBranchDetail.new('some_branch')).twice
     Runner.any_instance.stubs(:build_report_diff).twice
     SummaryReportBuilder.any_instance.stubs(:write_all_projects).once
 
     argv = %w[-r target/repositories/pmd -b master -bc config/design.xml -p pmd_releases/6.1.0
-              -pc config/design.xml -l test/resources/project-test.xml -t 4]
+              -pc config/design.xml -l test/resources/project-test.xml -t 5]
     run_runner(argv)
   end
 
@@ -101,13 +105,15 @@ def test_online_mode_multithreading
 
     report_builder_mock = mock
     PmdReportBuilder.stubs(:new)
-                    .with(anything, anything, anything, anything, 4)
+                    .with do |_, options, _, _|
+                      options.threads == 3
+                    end
                     .returns(report_builder_mock)
                     .once
     report_builder_mock.stubs(:build).returns(PmdBranchDetail.new('some_branch')).once
     SummaryReportBuilder.any_instance.stubs(:write_all_projects).once
 
-    argv = %w[-r target/repositories/pmd -m online -b master -p pmd_releases/6.7.0 -t 4]
+    argv = %w[-r target/repositories/pmd -m online -b master -p pmd_releases/6.7.0 -t 3]
     run_runner(argv)
   end
 end
diff --git a/test_out.txt b/test_out.txt
