Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support deterministic Cobertuna Reports #121

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import edu.hm.hafner.util.FilteredLog;
import edu.hm.hafner.util.PathUtil;
import edu.hm.hafner.util.SecureXmlParserFactory;
import edu.hm.hafner.util.TreeString;

Check warning on line 29 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / PMD

UnnecessaryImport

NORMAL: Unused import 'edu.hm.hafner.util.TreeString'.
Raw output
Reports import statements that can be removed. They are either unused, duplicated, or the members they import are already implicitly in scope, because they're in java.lang, or the current package. If some imports cannot be resolved, for instance because you run PMD with an incomplete auxiliary classpath, some imports may be conservatively marked as used even if they're not to avoid false positives. <pre> <code> import java.io.File; // not used, can be removed import java.util.Collections; // used below import java.util.*; // so this one is not used import java.lang.Object; // imports from java.lang, unnecessary import java.lang.Object; // duplicate, unnecessary public class Foo { static Object emptyList() { return Collections.emptyList(); } } </code> </pre> <a href="https://docs.pmd-code.org/pmd-doc-7.3.0/pmd_rules_java_codestyle.html#unnecessaryimport"> See PMD documentation. </a>

/**
* Parses Cobertura reports into a hierarchical Java Object Model.
Expand Down Expand Up @@ -140,17 +141,20 @@

private FileNode createFileNode(final StartElement element, final PackageNode packageNode) {
var fileName = getValueOf(element, FILE_NAME);
var path = getTreeStringBuilder().intern(PATH_UTIL.getRelativePath(fileName));

return packageNode.findOrCreateFileNode(getFileName(fileName), path);
var relativePath = PATH_UTIL.getRelativePath(fileName);
var actualPath = relativePath.startsWith("/_/") ?

Check warning on line 145 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 145 is only partially covered, one branch is missing

Check warning on line 145 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Mutation Coverage

Mutation survived

One mutation survived in line 145 (NegateConditionalsMutator)
Raw output
Survived mutations:
- negated conditional (org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator)

Check warning on line 145 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

NORMAL: (whitespace) OperatorWrap: '?' should be on a new line.

Check warning on line 145 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / CheckStyle

OperatorWrapCheck

NORMAL: '?' should be on a new line.
Raw output
<p>Since Checkstyle 3.0</p><p> Checks the policy on how to wrap lines on operators. </p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract "/_/" as a constant with a meaningful name (deterministic...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you use String#replaceFirst here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I've replaced the manual prefix check with String#replaceFirst to simplify the logic

relativePath.replace("/_/", "./") :

Check warning on line 146 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 146 is not covered by tests

Check warning on line 146 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

NORMAL: (whitespace) OperatorWrap: ':' should be on a new line.

Check warning on line 146 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / CheckStyle

OperatorWrapCheck

NORMAL: ':' should be on a new line.
Raw output
<p>Since Checkstyle 3.0</p><p> Checks the policy on how to wrap lines on operators. </p>
relativePath;
var finalPath = getTreeStringBuilder().intern(actualPath);
return packageNode.findOrCreateFileNode(getFileName(fileName), finalPath);
}

private String getFileName(final String relativePath) {
var path = Paths.get(PATH_UTIL.getAbsolutePath(relativePath)).getFileName();
if (path == null) {
return relativePath;
}
return path.toString();
var fileName = Paths.get(PATH_UTIL.getAbsolutePath(relativePath)).getFileName();
var actualFileName = (fileName != null && fileName.toString().startsWith("/_/")) ?

Check warning on line 154 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 154 is only partially covered, 2 branches are missing

Check warning on line 154 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Mutation Coverage

Mutation survived

2 mutations survived in line 154
Raw output
Survived mutations:
- negated conditional (org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator)
- negated conditional (org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator)

Check warning on line 154 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

NORMAL: (whitespace) OperatorWrap: '?' should be on a new line.

Check warning on line 154 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / CheckStyle

OperatorWrapCheck

NORMAL: '?' should be on a new line.
Raw output
<p>Since Checkstyle 3.0</p><p> Checks the policy on how to wrap lines on operators. </p>

Check warning on line 154 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / PMD

UnnecessaryLocalBeforeReturn

NORMAL: Consider simply returning the value vs storing it in local variable 'actualFileName'.
Raw output
Avoid the creation of unnecessary local variables <pre> <code> public class Foo { public int foo() { int x = doSomething(); return x; // instead, just &#x27;return doSomething();&#x27; } } </code> </pre> <a href="https://docs.pmd-code.org/pmd-doc-7.3.0/pmd_rules_java_codestyle.html#unnecessarylocalbeforereturn"> See PMD documentation. </a>
fileName.toString().replace("/_/", "./") :

Check warning on line 155 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 155 is not covered by tests

Check warning on line 155 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

NORMAL: (whitespace) OperatorWrap: ':' should be on a new line.

Check warning on line 155 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / CheckStyle

OperatorWrapCheck

NORMAL: ':' should be on a new line.
Raw output
<p>Since Checkstyle 3.0</p><p> Checks the policy on how to wrap lines on operators. </p>
(fileName != null ? fileName.toString() : relativePath);

Check warning on line 156 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 156 is only partially covered, one branch is missing

Check warning on line 156 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / PMD

UselessParentheses

NORMAL: Useless parentheses.
Raw output
Parenthesized expressions are used to override the default operator precedence rules. Parentheses whose removal would not change the relative nesting of operators are unnecessary, because they don't change the semantics of the enclosing expression. Some parentheses that strictly speaking are unnecessary, may still be considered useful for readability. This rule allows to ignore violations on two kinds of unnecessary parentheses: - "Clarifying" parentheses, which separate operators of difference precedence. While unnecessary, they make precedence rules explicit, which may be useful for rarely used operators. For example: ```java (a + b) & c // is equivalent to `a + b & c`, but probably clearer ``` Unset the property `ignoreClarifying` to report them. - "Balancing" parentheses, which are unnecessary but visually balance out another pair of parentheses around an equality operator. For example, those two expressions are equivalent: ```java (a == null) != (b == null) a == null != (b == null) ``` The parentheses on the right are required, and the parentheses on the left are just more visually pleasing. Unset the property `ignoreBalancing` to report them. <pre> <code> public class Foo { { int n = 0; n = (n); // here n = (n * 2) * 3; // and here n = n * (2 * 3); // and here } } </code> </pre> <a href="https://docs.pmd-code.org/pmd-doc-7.3.0/pmd_rules_java_codestyle.html#uselessparentheses"> See PMD documentation. </a>
return actualFileName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract the file name logic into a method and reuse it here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.CognitiveComplexity"})
Expand Down
Loading