Issue56510 #154
Issue56510 #154
Conversation
Codecov Report
@@ Coverage Diff @@
## master #154 +/- ##
===========================================
- Coverage 88.47% 88.38% -0.1%
- Complexity 1230 1260 +30
===========================================
Files 158 160 +2
Lines 3922 4045 +123
Branches 424 438 +14
===========================================
+ Hits 3470 3575 +105
- Misses 296 314 +18
Partials 156 156
Continue to review full report at Codecov.
|
|
Wasn't the idea to reuse the code in the warnings-plugin and the serialization of XStream? |
|
The reason why i didn't reuse the code is: "Adding dependency on module 'warnings-ng' will introduce circular dependency between modules 'analysis-model' and 'warnings-ng'. " |
|
Well, actually the idea was to place the parser class into the module warnings-ng in order to simply things. But now that you have done all the work it is fine to leave it here. Typically converting a bean to an XML or JSON file is just much simpler... |
| * | ||
| * @author Raphael Furch | ||
| */ | ||
| public class DefaultXmlMapper extends HashMap<String, String> { |
There was a problem hiding this comment.
Remove this class, keep it simple as possible.
| * | ||
| * @author Raphael Furch | ||
| */ | ||
| public class CustomXmlMapper extends HashMap<String, String> { |
| /** | ||
| * Identifier for file name. | ||
| */ | ||
| public static final String FILE_NAME = "fileName"; |
There was a problem hiding this comment.
Make all constants private and remove comment.
|
|
||
| /** | ||
| * Create a new ReportParser object. | ||
| * @param root = Path in xml file to issue collection. |
| Report report = new Report(); | ||
|
|
||
| NodeList issues = (NodeList)path.evaluate(getXmlIssueRoot(), doc, XPathConstants.NODESET); | ||
| // Read all issues. |
|
|
||
| /** | ||
| * Read a value from XML and set a default value if its null. | ||
| * @param path = xpath. |
There was a problem hiding this comment.
No = and reformat with IntelliJ auto format
| * @return value or default value. | ||
| * @throws XPathExpressionException error. | ||
| */ | ||
| private String notNullableRead(final XPath path, final String value, final Element issue) throws XPathExpressionException { |
There was a problem hiding this comment.
I think it is save to return null here as the IssueBuilder instance correctly handles null values.
| .hasReference("Reference 1") | ||
| .hasFingerprint("Fingerprint 1") | ||
| .hasAdditionalProperties("Property 1, Property 2"); | ||
|
|
There was a problem hiding this comment.
use auto format, just one blank line is enough
| } | ||
|
|
||
| @Test | ||
| void shouldParseWithCustomPathAndMapper() { |
| } | ||
|
|
||
| @Test | ||
| void shouldThrowParserException() { |
There was a problem hiding this comment.
Can you add one test that shows that the parser still produces a list of issues if just one XML tag contains wrong values?
Add a custom XML-Parser, which was inspired by the Eclipse XML parser.
https://issues.jenkins-ci.org/browse/JENKINS-56510