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

Unit tests for TooGenericExceptionThrown #4198

Merged
merged 2 commits into from
Oct 29, 2021
Merged

Unit tests for TooGenericExceptionThrown #4198

merged 2 commits into from
Oct 29, 2021

Conversation

Whathecode
Copy link
Contributor

  • The two unit tests for TooGenericExceptionThrown were testing the wrong rule.
  • Added a unit test to explicitely test that caught exceptions are excluded.
  • Added a unit test to recreate False positive TooGenericExceptionThrown #4197, which doesn't fail at the moment, so more information on this bug report is needed.

@codecov
Copy link

codecov bot commented Oct 23, 2021

Codecov Report

Merging #4198 (09ac1d7) into main (c5850c4) will increase coverage by 0.86%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #4198      +/-   ##
============================================
+ Coverage     83.43%   84.30%   +0.86%     
- Complexity     3149     3236      +87     
============================================
  Files           456      468      +12     
  Lines          9017    10185    +1168     
  Branches       1754     1786      +32     
============================================
+ Hits           7523     8586    +1063     
- Misses          567      659      +92     
- Partials        927      940      +13     
Impacted Files Coverage Δ
.../kotlin/io/gitlab/arturbosch/detekt/api/Context.kt 13.33% <0.00%> (-56.67%) ⬇️
...in/io/gitlab/arturbosch/detekt/api/SingleAssign.kt 83.33% <0.00%> (-16.67%) ⬇️
...b/detekt/tooling/internal/DefaultAnalysisResult.kt 87.50% <0.00%> (-12.50%) ⬇️
...lin/io/gitlab/arturbosch/detekt/api/ConfigAware.kt 50.00% <0.00%> (-10.00%) ⬇️
...b/arturbosch/detekt/rules/style/FileParsingRule.kt 92.30% <0.00%> (-7.70%) ⬇️
.../detekt/metrics/processors/ProjectSLOCProcessor.kt 92.85% <0.00%> (-7.15%) ⬇️
...bosch/detekt/rules/style/UnnecessaryInheritance.kt 92.85% <0.00%> (-7.15%) ⬇️
...ch/detekt/rules/coroutines/GlobalCoroutineUsage.kt 83.33% <0.00%> (-6.67%) ⬇️
...ab/arturbosch/detekt/rules/bugs/UnreachableCode.kt 93.75% <0.00%> (-6.25%) ⬇️
...rturbosch/detekt/rules/naming/VariableMaxLength.kt 93.75% <0.00%> (-6.25%) ⬇️
... and 346 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 c5850c4...09ac1d7. Read the comment docs.

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Could you add also the test you mentioned here: #4197 (comment)
So we could technically close #4197 till we have a new reproducer.

@Whathecode
Copy link
Contributor Author

Whathecode commented Oct 28, 2021

LGTM 👍 Could you add also the test you mentioned here: #4197 (comment) So we could technically close #4197 till we have a new reproducer.

I can, but the intent of the test I added was to reflect that use case in a clearer/descriptive way.

I see nothing different that happens in the provided source code, so in a way I think that test would be superfluous. I did test it to make sure, and it passed.

Let me know what you prefer:

  • Keep it as is.
  • Add it as a new test.
  • Replace """fun f() { val ex = Exception() }""" with the code reported in the issue, but keep the test name/a single test.

@BraisGabin BraisGabin merged commit 5d2471e into detekt:main Oct 29, 2021
@Whathecode Whathecode deleted the bugfix-4197 branch October 29, 2021 16:15
@cortinico cortinico added this to the 1.19.0 milestone Oct 31, 2021
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