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

Introduce TestRestTemplate Kotlin extensions #11039

Closed
wants to merge 1 commit into from
Closed

Introduce TestRestTemplate Kotlin extensions #11039

wants to merge 1 commit into from

Conversation

sdeleuze
Copy link
Contributor

This commit introduces Kotlin extensions similar to the RestOperations ones in order to be able to take advantage of Kotlin reified type parameters for example.

Fixes gh-8062

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 15, 2017
@philwebb philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 15, 2017
@philwebb philwebb added this to the 2.0.0.M7 milestone Nov 15, 2017
@@ -229,6 +229,25 @@
<artifactId>zt-zip</artifactId>
<version>1.7</version>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Dependencies are (meant to be) ordered alphabetically. Can you please rework this?

@@ -196,6 +211,38 @@
</execution>
</executions>
</plugin>
<plugin>
Copy link
Member

Choose a reason for hiding this comment

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

I am confused. In the spring-boot module you've set the maven compiler phases to none. Why isn't it necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it is, I will update the PR accordingly.

@sdeleuze
Copy link
Contributor Author

@snicoll I have pushed an updated commit with your feedback taken in account.

@snicoll
Copy link
Member

snicoll commented Nov 16, 2017

Thanks @sdeleuze. One more thing: we have a test that validates that our TestRestTemplate provides the same features as the original RestTemplate. Would it be possible to add a similar test to assert that this Kotlin extension is equivalent to the one for RestTemplate?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 16, 2017
@sdeleuze
Copy link
Contributor Author

@snicoll In Java, the test uses reflection to list all RestOperations methods and invoke their TestRestTemplate counterparts, but extensions are top level functions. Unless I miss something, I can see a reasonably complex way of doing the same kind of check with Kotlin extensions.

@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Nov 17, 2017
@snicoll snicoll self-assigned this Nov 17, 2017
@wilkinsona
Copy link
Member

@sdeleuze Perhaps we could do a similar but less extensive check? It would at least be nice to have a test that verifies that we have a top-level function for each method, even if the test doesn't invoke the function. I am working on the perhaps naive assumption that Kotlin is compiled to byte code so we must be able to introspect that byte code…

@sdeleuze
Copy link
Contributor Author

@wilkinsona But we don't have 1 extension for each method, we only provide the relevant ones.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 17, 2017

Sorry, "each method" was a little imprecise. I should have written "each method that's relevant". Assuming that we can identify a relevant method programatically, my hope is that it would be possible to test that there's a corresponding top-level function for that method.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 19, 2017
@snicoll snicoll removed their assignment Nov 19, 2017
@philwebb philwebb modified the milestones: 2.0.0.M7, 2.0.0.RC1 Nov 22, 2017
This commit introduces Kotlin extensions similar to
the RestOperations ones in order to be able to take
advantage of Kotlin reified type parameters for
example.

Fixes gh-8062
@sdeleuze
Copy link
Contributor Author

@snicoll @wilkinsona I have been able to create such test using a mix of Java and Kotlin reflection. It allowed to find 5 missing extensions, thanks for pushing for this ;-) I have added the missing extensions to this PR and to Spring Framework 5.0.2 as well via SPR-16229. Can you please merge this commit in order to be included as part of 2.0.0.M7?

@snicoll
Copy link
Member

snicoll commented Nov 24, 2017

Sébastien shared he'd like to use that feature during an upcoming workshop.

@snicoll snicoll self-assigned this Nov 24, 2017
@snicoll snicoll modified the milestones: 2.0.0.RC1, 2.0.0.M7 Nov 24, 2017
@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Nov 24, 2017
snicoll pushed a commit that referenced this pull request Nov 24, 2017
This commit introduces Kotlin extensions similar to the RestOperations
ones in order to be able to take advantage of Kotlin reified type
parameters for example.

See gh-11039
@snicoll snicoll closed this in aa87c45 Nov 24, 2017
snicoll added a commit that referenced this pull request Nov 24, 2017
* pr/11039:
  Polish "Introduce TestRestTemplate Kotlin extensions"
  Introduce TestRestTemplate Kotlin extensions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants