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

optimize: Changed exception check by JUnit API usage #3589

Merged
merged 5 commits into from
Mar 25, 2021

Conversation

eas5
Copy link
Contributor

@eas5 eas5 commented Mar 22, 2021

Problem:
The Exception Handling test smell occurs when the test outcome is manually determined through pass or fail method calls, dependent on the production method throwing an exception, generally inside a try/catch block. This refactoring proposal aims to make the exception catching a responsibility of the test framework, which is already provided by its API. Also, without using try/catch blocks, tests can be more straightforward and possibly more comprehensible and maintainable.

Solution:
We propose using JUnit's exception handling to automatically pass/fail the test instead of writing custom exception handling code. In this particular case, we added the possibly throwable object to the test method signature and removed the redundant fail method call and message.

Result:
Before:

void testCycleDependency() {
    CycleDependency A = CycleDependency.A;
    try {
        StringUtils.toString(A);
    } catch (StackOverflowError e) {
        Assertions.fail("stack overflow error");
    }
}

After:

void testCycleDependency() throws StackOverflowError{
    StringUtils.toString(CycleDependency.A);
}

Signed-off-by: Elvys Soares <eas5@cin.ufpe.br>
Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

LGTM

@caohdgege
Copy link
Contributor

plz modify the title of the issue to optimize: Changed exception check by JUnit API usage or test: Changed exception check by JUnit API usage
plz register your GitHub id & the issue info to the files under the changes folder

Copy link
Contributor

@caohdgege caohdgege left a comment

Choose a reason for hiding this comment

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

LGTM

@eas5 eas5 changed the title Changed exception check by JUnit API usage optimize: Changed exception check by JUnit API usage Mar 24, 2021
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

Codecov Report

Merging #3589 (f0e940d) into develop (d28274d) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head f0e940d differs from pull request most recent head 07dda37. Consider uploading reports for the commit 07dda37 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3589      +/-   ##
=============================================
- Coverage      52.08%   52.07%   -0.01%     
- Complexity      3499     3500       +1     
=============================================
  Files            638      638              
  Lines          21108    21108              
  Branches        2618     2617       -1     
=============================================
- Hits           10995    10993       -2     
- Misses          9027     9028       +1     
- Partials        1086     1087       +1     
Impacted Files Coverage Δ Complexity Δ
...torage/file/store/FileTransactionStoreManager.java 57.05% <0.00%> (-0.65%) 28.00% <0.00%> (-1.00%)

@xingfudeshi xingfudeshi merged commit 3c21b93 into apache:develop Mar 25, 2021
@funky-eyes funky-eyes added this to the 1.4.2 milestone Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants