Skip to content

Commit 4657173

Browse files
committed
Fix missing violations
When base or patch reports contain violations, that are duplicated in terms of eql? (same rule, same line, same message), then these were deduplicated when creating the union of all violations for a file. This resulted in odd numbers in the diff, e.g. less than all violations being removed when no violation were actually being left over.
1 parent 017eac8 commit 4657173

File tree

4 files changed

+47
-1
lines changed

4 files changed

+47
-1
lines changed

lib/pmdtester/report_diff.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ def build_diffs(counters, &getter)
211211
# Values are lists of violations/errors
212212
diffs = base_hash.to_h.merge(patch_hash.to_h) do |_key, base_value, patch_value|
213213
# make the difference of values
214-
(base_value | patch_value) - (base_value & patch_value)
214+
(base_value + patch_value) - (base_value & patch_value)
215215
end
216216

217217
diffs.delete_if do |_key, value|
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 https://pmd.github.io/schema/report_2_0_0.xsd"
5+
version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
6+
<file name="File1.java">
7+
<violation beginline="7" endline="8" begincolumn="1" endcolumn="2" rule="GenericNaming" ruleset="Codestyle" class="Null" method="Null" externalInfoUrl="http://pmd.sourceforge.net/snapshot/pmd_rules_java_codestyle.html#genericnaming" priority="3">
8+
Same message for the two violations on the same line.
9+
</violation>
10+
<violation beginline="7" endline="8" begincolumn="1" endcolumn="2" rule="GenericNaming" ruleset="Codestyle" class="Null" method="Null" externalInfoUrl="http://pmd.sourceforge.net/snapshot/pmd_rules_java_codestyle.html#genericnaming" priority="3">
11+
Same message for the two violations on the same line.
12+
</violation>
13+
</file>
14+
</pmd>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 https://pmd.github.io/schema/report_2_0_0.xsd"
5+
version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
6+
<file name="File1.java">
7+
<violation beginline="7" endline="8" begincolumn="1" endcolumn="2" rule="TypeParameterNamingConvention" ruleset="Codestyle" class="Null" method="Null" externalInfoUrl="http://pmd.sourceforge.net/snapshot/pmd_rules_java_codestyle.html#typeparameternamingconvention" priority="3">
8+
Message for first type name.
9+
</violation>
10+
<violation beginline="7" endline="8" begincolumn="1" endcolumn="2" rule="TypeParameterNamingConvention" ruleset="Codestyle" class="Null" method="Null" externalInfoUrl="http://pmd.sourceforge.net/snapshot/pmd_rules_java_codestyle.html#typeparameternamingconvention" priority="3">
11+
Message for second type name.
12+
</violation>
13+
</file>
14+
</pmd>

test/test_diff_builder.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ def test_violation_diffs
3737
# assert_equal('00:00:56', diffs_report.diff_execution_time)
3838
end
3939

40+
def test_violation_diffs_rule_message_change
41+
base_report_path = 'test/resources/diff_builder/test_violation_diffs_rule_message_change_base.xml'
42+
patch_report_path = 'test/resources/diff_builder/test_violation_diffs_rule_message_change_patch.xml'
43+
diffs_report = build_report_diff(base_report_path, patch_report_path,
44+
BASE_REPORT_INFO_PATH, PATCH_REPORT_INFO_PATH)
45+
violation_diffs = diffs_report.violation_diffs_by_file
46+
keys = violation_diffs.keys
47+
48+
assert_counters_empty(diffs_report.error_counts)
49+
assert_counters_eq(diffs_report.violation_counts,
50+
base_total: 2, patch_total: 2, changed_total: 4)
51+
assert_changes_eq(diffs_report.violation_counts,
52+
removed: 2, added: 2, changed: 0)
53+
54+
assert_equal('File1.java', keys[0])
55+
assert_equal(4, violation_diffs[keys[0]].size)
56+
end
57+
4058
def test_violation_diffs_with_filter
4159
base_report_path = 'test/resources/diff_builder/test_violation_diffs_base.xml'
4260
patch_report_path = 'test/resources/diff_builder/test_violation_diffs_patch.xml'

0 commit comments

Comments
 (0)