-
Notifications
You must be signed in to change notification settings - Fork 96
Correcting MockWebServer migration for OkHttp #849
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
base: main
Are you sure you want to change the base?
Conversation
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit5/UpgradeOkHttpMockWebServerTest.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit5/UpgradeOkHttpMockWebServerTest.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit5/UpgradeOkHttpMockWebServerTest.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/UpdateMockWebServerMockResponse.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit5/UpgradeOkHttpMockWebServerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit5/UpgradeOkHttpMockWebServerTest.java
Show resolved
Hide resolved
630c51b to
58ef5a9
Compare
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.openrewrite.java.Assertions.java; | ||
| import static org.openrewrite.java.Assertions.mavenProject; | ||
| import static org.openrewrite.java.Assertions.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import static org.openrewrite.java.Assertions.*; | |
| import static org.openrewrite.java.Assertions.java; | |
| import static org.openrewrite.java.Assertions.mavenProject; |
| } | ||
|
|
||
| @DocumentExample | ||
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? | |
| // @Test | |
| // void wip() { | |
| // rewriteRun( | |
| // //language=java | |
| // java( | |
| // """ | |
| // import okhttp3.Headers; | |
| // import okhttp3.mockwebserver.MockResponse; | |
| // | |
| // class A { | |
| // void someMethod() { | |
| // Headers headers = new Headers.Builder().build(); | |
| // MockResponse a = new MockResponse(); | |
| // // .status(String): void | |
| // // .getStatus(): String | |
| // // -- | |
| // // .setStatus(String): MockResponse[this] | |
| // // --- | |
| // // .headers(Headers): void | |
| // // .setHeaders(Headers): MockResponse | |
| // // .getHeaders(): Headers | |
| // // --- | |
| // // .addHeader(String): MockResponse | |
| // // .addHeader(String,Object): MockResponse | |
| // // .addHeaderLenient(String,Object): MockResponse | |
| // // --- | |
| // // .setHeader(String,Object): MockResponse | |
| // // .removeHeader(String): MockResponse | |
| // // .clearHeaders(): MockResponse | |
| // a.header | |
| // a.trailers(headers); | |
| // } | |
| // } | |
| // """ | |
| // ) | |
| // ); | |
| // } | |
| @DocumentExample | |
| @Test | |
| void shouldUpgradeMavenDependency() { | |
| rewriteRun( | |
| spec -> spec.recipeFromResource("/META-INF/rewrite/junit5.yml", "org.openrewrite.java.testing.junit5.UpgradeOkHttpMockWebServer"), | |
| mavenProject("project", | |
| // TODO: handle solely J.NewClass and update declarative recipe to include new one. | |
| //language=xml | |
| pomXml( | |
| """ | |
| <project> | |
| <modelVersion>4.0.0</modelVersion> | |
| <groupId>com.example</groupId> | |
| <artifactId>demo</artifactId> | |
| <version>0.0.1-SNAPSHOT</version> | |
| <dependencies> | |
| <dependency> | |
| <groupId>com.squareup.okhttp3</groupId> | |
| <artifactId>mockwebserver</artifactId> | |
| <version>4.10.0</version> | |
| <scope>test</scope> | |
| </dependency> | |
| </dependencies> | |
| </project> | |
| """, | |
| spec -> spec.after(pom -> | |
| assertThat(pom) | |
| .doesNotContain("<artifactId>mockwebserver</artifactId>") | |
| .contains("<artifactId>mockwebserver3</artifactId>") | |
| .containsPattern("<version>5\\.(.*)</version>") | |
| .actual() | |
| ) | |
| ) | |
| ) | |
| ); | |
| } | |
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
|
I'm going to try to look into whether I can set up a single visitor to perform all the type changes needed prior to using the |
58ef5a9 to
9f40caf
Compare
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.openrewrite.java.Assertions.java; | ||
| import static org.openrewrite.java.Assertions.mavenProject; | ||
| import static org.openrewrite.java.Assertions.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import static org.openrewrite.java.Assertions.*; | |
| import static org.openrewrite.java.Assertions.java; | |
| import static org.openrewrite.java.Assertions.mavenProject; |
| } | ||
|
|
||
| @DocumentExample | ||
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? | |
| // @Test | |
| // void wip() { | |
| // rewriteRun( | |
| // //language=java | |
| // java( | |
| // """ | |
| // import okhttp3.Headers; | |
| // import okhttp3.mockwebserver.MockResponse; | |
| // | |
| // class A { | |
| // void someMethod() { | |
| // Headers headers = new Headers.Builder().build(); | |
| // MockResponse a = new MockResponse(); | |
| // // .status(String): void | |
| // // .getStatus(): String | |
| // // -- | |
| // // .setStatus(String): MockResponse[this] | |
| // // --- | |
| // // .headers(Headers): void | |
| // // .setHeaders(Headers): MockResponse | |
| // // .getHeaders(): Headers | |
| // // --- | |
| // // .addHeader(String): MockResponse | |
| // // .addHeader(String,Object): MockResponse | |
| // // .addHeaderLenient(String,Object): MockResponse | |
| // // --- | |
| // // .setHeader(String,Object): MockResponse | |
| // // .removeHeader(String): MockResponse | |
| // // .clearHeaders(): MockResponse | |
| // a.header | |
| // a.trailers(headers); | |
| // } | |
| // } | |
| // """ | |
| // ) | |
| // ); | |
| // } | |
| @DocumentExample | |
| @Test | |
| void shouldUpgradeMavenDependency() { | |
| rewriteRun( | |
| spec -> spec.recipeFromResource("/META-INF/rewrite/junit5.yml", "org.openrewrite.java.testing.junit5.UpgradeOkHttpMockWebServer"), | |
| mavenProject("project", | |
| // TODO: handle solely J.NewClass and update declarative recipe to include new one. | |
| //language=xml | |
| pomXml( | |
| """ | |
| <project> | |
| <modelVersion>4.0.0</modelVersion> | |
| <groupId>com.example</groupId> | |
| <artifactId>demo</artifactId> | |
| <version>0.0.1-SNAPSHOT</version> | |
| <dependencies> | |
| <dependency> | |
| <groupId>com.squareup.okhttp3</groupId> | |
| <artifactId>mockwebserver</artifactId> | |
| <version>4.10.0</version> | |
| <scope>test</scope> | |
| </dependency> | |
| </dependencies> | |
| </project> | |
| """, | |
| spec -> spec.after(pom -> | |
| assertThat(pom) | |
| .doesNotContain("<artifactId>mockwebserver</artifactId>") | |
| .contains("<artifactId>mockwebserver3</artifactId>") | |
| .containsPattern("<version>5\\.(.*)</version>") | |
| .actual() | |
| ) | |
| ) | |
| ) | |
| ); | |
| } | |
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
…padding (including the newline), leading to it changing wrapping.
…possible for future error occurrences. (#853)
…uild()`, and backpatching the parameter types onto the various method invocations. Feels like there could be a lot of improvement still with how this done, as it feels pretty hacky right now.
9f40caf to
7855d82
Compare
| import static java.util.Collections.emptySet; | ||
| import static java.util.Objects.requireNonNull; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import static java.util.Collections.emptySet; | |
| import static java.util.Objects.requireNonNull; | |
| import static java.util.Collections.emptySet; |
| private static final String OLD_MOCKRESPONSE_CONSTRUCTOR = OLD_MOCKRESPONSE_FQN + " <constructor>()"; | ||
| private static final String OLD_MOCKRESPONSE_STATUS = OLD_MOCKRESPONSE_FQN + " status(java.lang.String)"; | ||
| private static final String OLD_MOCKRESPONSE_SETSTATUS = OLD_MOCKRESPONSE_FQN + " setStatus(java.lang.String)"; | ||
| private static final String OLD_MOCKRESPONSE_HEADERS = OLD_MOCKRESPONSE_FQN + " headers(okhttp3.Headers)"; | ||
| private static final String OLD_MOCKRESPONSE_SETHEADERS = OLD_MOCKRESPONSE_FQN + " setHeaders(okhttp3.Headers)"; | ||
| private static final String NEW_MOCKRESPONSE_FQN = NEW_PACKAGE_NAME + ".MockResponse"; | ||
| // TODO: Rename this given this actually includes '$' | ||
| private static final String NEW_MOCKRESPONSE_FQN_BUILDER = NEW_MOCKRESPONSE_FQN + "$Builder"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static final String OLD_MOCKRESPONSE_CONSTRUCTOR = OLD_MOCKRESPONSE_FQN + " <constructor>()"; | |
| private static final String OLD_MOCKRESPONSE_STATUS = OLD_MOCKRESPONSE_FQN + " status(java.lang.String)"; | |
| private static final String OLD_MOCKRESPONSE_SETSTATUS = OLD_MOCKRESPONSE_FQN + " setStatus(java.lang.String)"; | |
| private static final String OLD_MOCKRESPONSE_HEADERS = OLD_MOCKRESPONSE_FQN + " headers(okhttp3.Headers)"; | |
| private static final String OLD_MOCKRESPONSE_SETHEADERS = OLD_MOCKRESPONSE_FQN + " setHeaders(okhttp3.Headers)"; | |
| private static final String NEW_MOCKRESPONSE_FQN = NEW_PACKAGE_NAME + ".MockResponse"; | |
| // TODO: Rename this given this actually includes '$' | |
| private static final String NEW_MOCKRESPONSE_FQN_BUILDER = NEW_MOCKRESPONSE_FQN + "$Builder"; | |
| private static final String OLD_MOCKRESPONSE_CONSTRUCTOR = OLD_MOCKRESPONSE_FQN + " <constructor>()"; |
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.openrewrite.java.Assertions.java; | ||
| import static org.openrewrite.java.Assertions.mavenProject; | ||
| import static org.openrewrite.java.Assertions.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import static org.openrewrite.java.Assertions.*; | |
| import static org.openrewrite.java.Assertions.java; | |
| import static org.openrewrite.java.Assertions.mavenProject; |
| } | ||
|
|
||
| @DocumentExample | ||
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? | |
| // @Test | |
| // void wip() { | |
| // rewriteRun( | |
| // //language=java | |
| // java( | |
| // """ | |
| // import okhttp3.Headers; | |
| // import okhttp3.mockwebserver.MockResponse; | |
| // | |
| // class A { | |
| // void someMethod() { | |
| // Headers headers = new Headers.Builder().build(); | |
| // MockResponse a = new MockResponse(); | |
| // // .status(String): void | |
| // // .getStatus(): String | |
| // // -- | |
| // // .setStatus(String): MockResponse[this] | |
| // // --- | |
| // // .headers(Headers): void | |
| // // .setHeaders(Headers): MockResponse | |
| // // .getHeaders(): Headers | |
| // // --- | |
| // // .addHeader(String): MockResponse | |
| // // .addHeader(String,Object): MockResponse | |
| // // .addHeaderLenient(String,Object): MockResponse | |
| // // --- | |
| // // .setHeader(String,Object): MockResponse | |
| // // .removeHeader(String): MockResponse | |
| // // .clearHeaders(): MockResponse | |
| // a.header | |
| // a.trailers(headers); | |
| // } | |
| // } | |
| // """ | |
| // ) | |
| // ); | |
| // } | |
| @DocumentExample | |
| @Test | |
| void shouldUpgradeMavenDependency() { | |
| rewriteRun( | |
| spec -> spec.recipeFromResource("/META-INF/rewrite/junit5.yml", "org.openrewrite.java.testing.junit5.UpgradeOkHttpMockWebServer"), | |
| mavenProject("project", | |
| // TODO: handle solely J.NewClass and update declarative recipe to include new one. | |
| //language=xml | |
| pomXml( | |
| """ | |
| <project> | |
| <modelVersion>4.0.0</modelVersion> | |
| <groupId>com.example</groupId> | |
| <artifactId>demo</artifactId> | |
| <version>0.0.1-SNAPSHOT</version> | |
| <dependencies> | |
| <dependency> | |
| <groupId>com.squareup.okhttp3</groupId> | |
| <artifactId>mockwebserver</artifactId> | |
| <version>4.10.0</version> | |
| <scope>test</scope> | |
| </dependency> | |
| </dependencies> | |
| </project> | |
| """, | |
| spec -> spec.after(pom -> | |
| assertThat(pom) | |
| .doesNotContain("<artifactId>mockwebserver</artifactId>") | |
| .contains("<artifactId>mockwebserver3</artifactId>") | |
| .containsPattern("<version>5\\.(.*)</version>") | |
| .actual() | |
| ) | |
| ) | |
| ) | |
| ); | |
| } | |
| // TODO: methods receiving MockResponse - maybe add comment instructing to double check? |
What's changed?
mockwebserver3-junit5tomockwebserver3MockResponse. Now everything has to be switched to usingMockResponse.Builderto change all these values.What's your motivation?
Compilation failures after previous setup's migration ran.
Anything in particular you'd like reviewers to focus on?
Checklist