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

JUnitAssertThrowsToAssertExceptionType does not convert cases where executable is a variable #511

Open
timtebeek opened this issue Apr 23, 2024 · 2 comments
Labels
assertj bug Something isn't working

Comments

@timtebeek
Copy link
Contributor

timtebeek commented Apr 23, 2024

What version of OpenRewrite are you using?

What is the smallest, simplest way to reproduce the problem?

import org.junit.jupiter.api.function.Executable;

import static org.junit.jupiter.api.Assertions.assertThrows;

public class SimpleExpectedExceptionTest {
    public void throwsExceptionWithSpecificType() {
        Executable executable = () -> {
            throw new NullPointerException();
        };
        assertThrows(NullPointerException.class, executable);
    }
}

What did you expect to see?

import org.assertj.core.api.ThrowableAssert.ThrowingCallable;

import static org.junit.jupiter.api.Assertions.assertThrows;

public class SimpleExpectedExceptionTest {
    public void throwsExceptionWithSpecificType() {
        ThrowingCallable executable = () -> {
            throw new NullPointerException();
        };
        assertThatExceptionOfType(NullPointerException.class).isThrownBy(executable);
    }
}

What did you see instead?

No change, as the recipe fails to match the case where the argument is a variable.

J executable = mi.getArguments().get(1);
if (executable instanceof J.Lambda) {
executable = ((J.Lambda) executable).withType(THROWING_CALLABLE_TYPE);
} else if (executable instanceof J.MemberReference) {
executable = ((J.MemberReference) executable).withType(THROWING_CALLABLE_TYPE);
} else {
executable = null;

Additional context

As reported on

@timtebeek timtebeek added the bug Something isn't working label Apr 23, 2024
@timtebeek timtebeek added the good first issue Good for newcomers label Apr 23, 2024
@timtebeek timtebeek moved this to Recipes Wanted in OpenRewrite Apr 23, 2024
@Riyazul555
Copy link

I guess instead of setting executable to null if it’s not a Lambda or MemberReference, like this

J executable = mi.getArguments().get(1);
                if (executable instanceof J.Lambda) {
                    executable = ((J.Lambda) executable).withType(THROWING_CALLABLE_TYPE);
                } else if (executable instanceof J.MemberReference) {
                    executable = ((J.MemberReference) executable).withType(THROWING_CALLABLE_TYPE);
                } else {
                    executable = executable.withType(THROWING_CALLABLE_TYPE);
                }

the code now sets the type of executable directly. This allows variables to be used correctly.
The withType(THROWING_CALLABLE_TYPE) method is applied to all types of Executable, ensuring the transformation applies universally and keeping the method signature and descriptions concise and relevant. This change should handle cases where the Executable is a variable in the assertThrows call.

What are your thoughts @timtebeek ??

Laurens-W added a commit that referenced this issue Aug 8, 2024
@timtebeek timtebeek removed the good first issue Good for newcomers label Aug 12, 2024
@timtebeek
Copy link
Contributor Author

We've had another look at this, but this seems harder than initially anticipated. We'd have to track assignments and change their type to make this work. Removing the good-first-issue label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assertj bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants