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

Add complexity report to html output #2071

Merged

Conversation

91pratyush
Copy link
Contributor

@91pratyush 91pratyush commented Oct 31, 2019

This PR is with respect to issue discussed at #2046 .
Currently, Complexity report is only printed as part of console output.
This PR attempts to push complexity report as part of the HTML output in a <div> container.

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The product code looks good.
The only thing that's left is the unit test part in order to maintain the quality of detekt.

@@ -34,6 +34,7 @@ class HtmlOutputFormatTest : Spek({

assertThat(result).contains("<h1>detekt report</h1>")
assertThat(result).contains("<h2>Metrics</h2>")
assertThat(result).contains("<h2>Complexity Report</h2>")
Copy link
Member

Choose a reason for hiding this comment

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

Tests are missing for the Metrics and Complexity Report where there's actually an item in the list (<li></li>).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added these tests.

Copy link
Member

Choose a reason for hiding this comment

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

1 unit test is missing for the case when the complexity report is null or blank and thus returns an empty string list.
The rest looks good. Thank you! :-)

if (complexityReport.isNullOrBlank()) listOf<String>()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. Added the missing test. Thank you :)

@codecov-io
Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Merging #2071 into master will decrease coverage by 0.05%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2071      +/-   ##
============================================
- Coverage     80.59%   80.54%   -0.06%     
- Complexity     1988     2006      +18     
============================================
  Files           332      336       +4     
  Lines          5705     5776      +71     
  Branches       1042     1061      +19     
============================================
+ Hits           4598     4652      +54     
- Misses          554      563       +9     
- Partials        553      561       +8
Impacted Files Coverage Δ Complexity Δ
...tlab/arturbosch/detekt/cli/out/HtmlOutputReport.kt 91.8% <88.88%> (+1.41%) 17 <4> (+4) ⬆️
...turbosch/detekt/api/internal/SimpleNotification.kt 50% <0%> (-50%) 1% <0%> (ø)
...osch/detekt/rules/style/OptionalAbstractKeyword.kt 83.33% <0%> (-8.34%) 6% <0%> (ø)
...n/kotlin/io/gitlab/arturbosch/detekt/api/Entity.kt 84.61% <0%> (-7.7%) 4% <0%> (-1%)
...tlin/io/gitlab/arturbosch/detekt/api/YamlConfig.kt 82.6% <0%> (-3.76%) 8% <0%> (ø)
...b/arturbosch/detekt/rules/complexity/LargeClass.kt 93.75% <0%> (-3.13%) 12% <0%> (ø)
...rbosch/detekt/rules/complexity/TooManyFunctions.kt 96.77% <0%> (-1.62%) 26% <0%> (ø)
.../io/gitlab/arturbosch/detekt/cli/runners/Runner.kt 87.8% <0%> (-1.49%) 9% <0%> (+3%)
...kotlin/io/gitlab/arturbosch/detekt/api/Findings.kt 60% <0%> (ø) 0% <0%> (ø) ⬇️
...ain/kotlin/io/gitlab/arturbosch/detekt/cli/Main.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
... and 16 more

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 72f2225...da374c6. Read the comment docs.

@schalkms schalkms merged commit 85c93ea into detekt:master Nov 5, 2019
@schalkms
Copy link
Member

schalkms commented Nov 5, 2019

Thanks! This is an awesome contribution.

@arturbosch arturbosch added this to the 1.2.0 milestone Nov 5, 2019
@91pratyush
Copy link
Contributor Author

Nevermind. It was a pleasure 😄

smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Add complexity report summary to html output

* Fixes to conform to kotlin codestyle guidelines

* Push complexity report as list instead of text blob

* Addressing PR comments and added tests for metrics and complexity in html output

* refactor to get rid of detekt warning

* Add test case to check when complexity report is null or blank
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Add complexity report summary to html output

* Fixes to conform to kotlin codestyle guidelines

* Push complexity report as list instead of text blob

* Addressing PR comments and added tests for metrics and complexity in html output

* refactor to get rid of detekt warning

* Add test case to check when complexity report is null or blank
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.

None yet

4 participants