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

[JENKINS-57098] Add JSON Parser #168

Merged
merged 6 commits into from
May 20, 2019
Merged

Conversation

jmini
Copy link
Contributor

@jmini jmini commented May 6, 2019

@codecov
Copy link

codecov bot commented May 6, 2019

Codecov Report

Merging #168 into master will increase coverage by 0.03%.
The diff coverage is 90.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #168      +/-   ##
===========================================
+ Coverage     88.37%   88.4%   +0.03%     
- Complexity     1293    1320      +27     
===========================================
  Files           161     162       +1     
  Lines          4146    4209      +63     
  Branches        441     464      +23     
===========================================
+ Hits           3664    3721      +57     
- Misses          327     328       +1     
- Partials        155     160       +5
Impacted Files Coverage Δ Complexity Δ
...java/edu/hm/hafner/analysis/parser/JsonParser.java 90.47% <90.47%> (ø) 27 <27> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97739e7...f3fedeb. Read the comment docs.

@codecov
Copy link

codecov bot commented May 6, 2019

Codecov Report

Merging #168 into master will increase coverage by 0.05%.
The diff coverage is 92.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #168      +/-   ##
============================================
+ Coverage     88.37%   88.43%   +0.05%     
- Complexity     1293     1322      +29     
============================================
  Files           161      162       +1     
  Lines          4146     4210      +64     
  Branches        441      464      +23     
============================================
+ Hits           3664     3723      +59     
  Misses          327      327              
- Partials        155      160       +5
Impacted Files Coverage Δ Complexity Δ
src/main/java/edu/hm/hafner/analysis/Issue.java 75.3% <ø> (ø) 65 <0> (ø) ⬇️
.../java/edu/hm/hafner/analysis/parser/XmlParser.java 93.1% <100%> (+0.12%) 12 <0> (+1) ⬆️
...java/edu/hm/hafner/analysis/parser/JsonParser.java 90.47% <90.47%> (ø) 27 <27> (?)
...java/edu/hm/hafner/analysis/FileReaderFactory.java 82.14% <0%> (+3.57%) 10% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97739e7...78866a4. Read the comment docs.

@codecov-io
Copy link

Codecov Report

Merging #168 into master will increase coverage by 0.03%.
The diff coverage is 92.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #168      +/-   ##
============================================
+ Coverage      88.4%   88.43%   +0.03%     
- Complexity     1295     1322      +27     
============================================
  Files           161      162       +1     
  Lines          4147     4210      +63     
  Branches        441      464      +23     
============================================
+ Hits           3666     3723      +57     
- Misses          326      327       +1     
- Partials        155      160       +5
Impacted Files Coverage Δ Complexity Δ
src/main/java/edu/hm/hafner/analysis/Issue.java 75.3% <ø> (ø) 65 <0> (ø) ⬇️
.../java/edu/hm/hafner/analysis/parser/XmlParser.java 93.1% <100%> (ø) 12 <0> (ø) ⬇️
...java/edu/hm/hafner/analysis/parser/JsonParser.java 90.47% <90.47%> (ø) 27 <27> (?)
...va/edu/hm/hafner/analysis/parser/MentorParser.java 98.14% <0%> (ø) 15% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2f213b...5558b6c. Read the comment docs.

1 similar comment
@codecov-io
Copy link

Codecov Report

Merging #168 into master will increase coverage by 0.03%.
The diff coverage is 92.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #168      +/-   ##
============================================
+ Coverage      88.4%   88.43%   +0.03%     
- Complexity     1295     1322      +27     
============================================
  Files           161      162       +1     
  Lines          4147     4210      +63     
  Branches        441      464      +23     
============================================
+ Hits           3666     3723      +57     
- Misses          326      327       +1     
- Partials        155      160       +5
Impacted Files Coverage Δ Complexity Δ
src/main/java/edu/hm/hafner/analysis/Issue.java 75.3% <ø> (ø) 65 <0> (ø) ⬇️
.../java/edu/hm/hafner/analysis/parser/XmlParser.java 93.1% <100%> (ø) 12 <0> (ø) ⬇️
...java/edu/hm/hafner/analysis/parser/JsonParser.java 90.47% <90.47%> (ø) 27 <27> (?)
...va/edu/hm/hafner/analysis/parser/MentorParser.java 98.14% <0%> (ø) 15% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2f213b...5558b6c. Read the comment docs.

@jmini
Copy link
Contributor Author

jmini commented May 17, 2019

The revapi:update-versions plugin is suggesting a version bump to 5.1.0-SNAPSHOT. Should I add this commit to this PR to make the CI-job happy?

@uhafner uhafner merged commit 5558b6c into jenkinsci:master May 20, 2019
@uhafner
Copy link
Member

uhafner commented May 20, 2019

Thanks!

Should I simply add it as a second parser to WarningsPlugin. In the help we can briefly describe the two formats and link to the documentation?

@jmini
Copy link
Contributor Author

jmini commented May 21, 2019

Thank you a lot for merging this!

I would appreciate that the parser can be called from the "Jenkins Warnings Next Generation Plugin", I am not sure about how it can be integrated.


While I was looking at the documentation (details of the analysis result), I have seen that the plugin is capable of exporting existing issues as JSON report with:

GET [build-url]/[tool-id]/all/api/json

You get something like this:

{
    "_class": "io.jenkins.plugins.analysis.core.restapi.ReportApi",
    "issues": [
        {
            "baseName": "file.txt",
            "category": "Text-check",
            "columnEnd": 0,
            "columnStart": 0,
            "description": "Some error 1",
            "fileName": "/______/.jenkins/workspace/test/file.adoc",
            "fingerprint": "9CED6585900DD3CFB97B914A3CEB0E79",
            "lineEnd": 5,
            "lineStart": 5,
            "message": "Some error 1 message",
            "moduleName": "",
            "origin": "customChecker",
            "packageName": "-",
            "reference": "10",
            "severity": "ERROR",
            "type": "-"
        },
        {
            "baseName": "file2.txt",
            "category": "Text-check",
            "columnEnd": 0,
            "columnStart": 0,
            "description": "Some error 2",
            "fileName": "/______/.jenkins/workspace/test/file2.adoc",
            "fingerprint": "CE60859A425B0EC230F79F554F85C254",
            "lineEnd": 6,
            "lineStart": 6,
            "message": "Some error 2 message",
            "moduleName": "",
            "origin": "customChecker",
            "packageName": "-",
            "reference": "10",
            "severity": "ERROR",
            "type": "-"
        }
    ],
    "size": 2
}

I think that I should create a new PR to add a parser that is capable of reading this kind of report:

  • Read JSON File.
  • Look for an issues array of issues (for the conversion JSONObject -> Issue the code should be shared, maybe in some abstract class).

This way we would have 2 different JSON cases:

  • One (discussed above - to be created) expecting a valid JSON file (exported by the plugin). I think this corresponds to an additional option to "Warnings Plugin Native Format" you have described in your comment.
  • What I have contributed in this PR: the possibility to read json entries stored in each line of a log file (what I would like to use for the Asciidoctor export).

@uhafner: What do you think?

@uhafner
Copy link
Member

uhafner commented May 21, 2019

Yes, it would be helpful to have such a parser as well.

We can also add the parser from this PR as third alternative to the tool WarningsPlugin. Then we have support for:

  1. *.xml: by XmlParser
  2. *.json: by NotYetWrittenParser
  3. . (no *.xml and no. *.json): by JsonParser (from this PR).

The selection of the correct parser is automatically done by the file extension.

Or would you rather prefer to define a new tool Asciidoctor that internally uses the JsonParser?

@jmini
Copy link
Contributor Author

jmini commented May 21, 2019

+1

Maybe I should rename the current JsonParser from this PR to JsonLogParser or LogJsonParser (your point "3.") and create a new JsonFileParser (your point "2.").

Do I need to open a new JIRA issue for my next PR?

@uhafner
Copy link
Member

uhafner commented May 21, 2019

Maybe I should rename the current JsonParser from this PR to JsonLogParser or LogJsonParser (your point "3.") and create a new JsonFileParser (your point "2.").

That would make sense, yes. Then we can reuse the JsonParser for 2.

Do I need to open a new JIRA issue for my next PR?

No, just go ahead.

@jmini
Copy link
Contributor Author

jmini commented May 22, 2019

See PR #177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants