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 testcases to validate switch cases in SystemErrToLogging and ParameterizedLogging #198

Merged
merged 5 commits into from
Dec 5, 2024

Conversation

Laurens-W
Copy link
Contributor

What's changed?

Add testcase to validate switch cases in SystemErrToLogging and ParameterizedLogging

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

@timtebeek

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Sorry, something went wrong.

@Laurens-W Laurens-W added the bug Something isn't working label Dec 5, 2024
@Laurens-W Laurens-W requested a review from timtebeek December 5, 2024 10:56
@Laurens-W Laurens-W self-assigned this Dec 5, 2024
@timtebeek
Copy link
Contributor

No failure to run any more, but now seeing some unexpected outcomes.

SystemErrToLoggingTest > useSlf4j() FAILED
    org.opentest4j.AssertionFailedError: [Unexpected result in "Test.java":
    diff --git a/Test.java b/Test.java
    index 0637b38..886db3b 100644
    --- a/Test.java
    +++ b/Test.java
    @@ -6,7 +6,8 @@ 
         void test() {
             try {
             } catch(Throwable t) {
    -            logger.error("Oh {} no", n, t);
    +            logger.error("Oh {} no", n);
    +            t.printStackTrace();
             }
         }
     }
    \ No newline at end of file
    ] 

@timtebeek
Copy link
Contributor

@Laurens-W I've taken out the firstEnclosing instanceof J.Block || you had added to make the original tests pass again; do we need another test to assert the correct handling there based on the failure we saw above?

@Laurens-W
Copy link
Contributor Author

@Laurens-W I've taken out the firstEnclosing instanceof J.Block || you had added to make the original tests pass again; do we need another test to assert the correct handling there based on the failure we saw above?

Looking at it again I realized Case needed something similar to the visitor present for Block, the exceptional case for Lambda is because a Lambda can either be a single Statement or fall into the Block visitor

@timtebeek timtebeek merged commit d7fb6d0 into main Dec 5, 2024
2 checks passed
@timtebeek timtebeek deleted the validate-switch-template-pt2 branch December 5, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants