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

Cannot reference Stub Runner random port in Feign Client #1303

Closed
akutschera opened this issue Jan 4, 2020 · 14 comments
Closed

Cannot reference Stub Runner random port in Feign Client #1303

akutschera opened this issue Jan 4, 2020 · 14 comments
Labels

Comments

@akutschera
Copy link

I have a FeignClient interface that I want to test against my stubs.
@FeignClient(name = "demo", url = "${test.url}")
I would like the stubs to run on a random port

@SpringBootTest(properties = { 
   "test.url=http://localhost:${stubrunner.runningstubs.test-webapp.port:8082}" })
@AutoConfigureStubRunner(stubsMode = CLASSPATH, 
   ids = { "com.example:test-webapp:+:stubs" })

#147 and #1225 suggest that this works. My tests fail because the client tries to connect to port 8082 and the server is started on some random port.

I tried this with SB 2.2.2 and Hoxton.SR1 (see attached zip file)

What am I missing?

demo.zip

@marcingrzejszczak
Copy link
Contributor

That was supposed to be working. I guess I was too fast to state that. Thanks for the demo, I'll take a look at that.

@akutschera
Copy link
Author

When I turn on trace debugging, I see that the DemoFeignClient gets created just before the 'org.springframework.cloud.contract.stubrunner.spring.StubRunnerPortBeanPostProcessor'
is created.
If the StubRunner that is declared in the test would get created before the Client that is declared in src/main this would probably work.
Is there any way I can change the order of bean creation for the test so this works?
BTW, this is with SB 2.3.1 and Hoxton.SR6, too

@lukasz-gryzbon
Copy link

This issue is preventing my entire team from implementing CDC, unfortunately.

Just to clarify, our current working tests use @AutoConfigureWireMock(port = 0) and in the application-test.properties we inject the ${wiremock.server.port} into the url property that the Feign clients use to connect to the Wiremock.

As soon as we add @AutoConfigureStubRunner set to use a random port (and disable the stubs written manually), everything falls apart because there is no way to inject the port number to the url property.

Using a fixed port on @AutoConfigureStubRunner marks the context as dirty after each test which causes the tests to run extremly slowely and we can't accept this.

We use Feign clients only so it seems like there is no way for us to use Spring Cloud Contract, which is a huge shame.

Is there anything we could do to help fix this issue?

@marcingrzejszczak
Copy link
Contributor

I remember that I started to work on this some time ago. The fix is not trivial... The problem lays in Feign not in Spring Cloud Contract. AFAIR Feign is registering its beans early and eagerly - so it can't understand the provided stub runner property. I'll try to look into this again.

As a workaround, what you can do is something of the following:

  • Use a Stub Runner rule (here the stubs are started without the Spring context)
  • Run a test with a manual Spring context initalization (e.g. via ApplicationContextRunner) - here the context is started and the stubs are already there
  • Add the property test.url with already resolved port - you can do it since the stubs are already resolved and you know their ports

I know that it's not perfect but at least it's a start.

@lukasz-gryzbon
Copy link

Thank for your reply @marcingrzejszczak

I wonder what the relation between @AutoConfigureWireMock and @AutoConfigureStubRunner is. The reasons why I am thinking about it are:

  1. When we use @AutoConfigureWireMock with a random port, it populates the wiremock.server.port env value before the Feign clients are created. So maybe @AutoConfigureStubRunner should try to follow and implement a similar solution?
  2. In fact, since Spring Cloud already has the means to manage the Wirmock server (@AutoConfigureWireMock), maybe this functionality could/should be removed from @AutoConfigureStubRunner and allow it to either detect the Wiremock port or receive it as a parameter, eg. @AutoConfigureStubRunner(port="${wiremock.server.port"}). To me it seems counterintuitive to have these two annotations with overlapping functionality, but admittedly, I don't know the full stories behind either of the projects.

What do you think?

@marcingrzejszczak
Copy link
Contributor

In fact, since Spring Cloud already has the means to manage the Wirmock server (@AutoConfigureWireMock), maybe this functionality could/should be removed from @AutoConfigureStubRunner and allow it to either detect the Wiremock port or receive it as a parameter, eg. @AutoConfigureStubRunner(port="${wiremock.server.port"}). To me it seems counterintuitive to have these two annotations with overlapping functionality, but admittedly, I don't know the full stories behind either of the projects.

@AutoConfigureWireMock has nothing to do with @AutoConfiureStubRunner. Of course I looked at the original issue where it was written that one is using @AutoConfigureStubRunner, however you're using WireMock only from what I see. I'm a little bit confused. Can you share your sample so that I can get the full picture?

@lukasz-gryzbon
Copy link

lukasz-gryzbon commented Jan 11, 2021

We currently use @AutoConfigureWireMock, but the proof of concept I am working on doesn't. From what I understand, assuming there are no other subs involved, it's either or - no point in having both annotations.

The POC looks like this at the moment (I have removed manually everything I believe irrelevant, so hopefully there won't any silly mistakes):

@SpringBootTest(webEnvironment = DEFINED_PORT, classes = SomeApplication.class)
@AutoConfigureStubRunner(stubsMode = REMOTE, ids = "com.example:some-service", repositoryRoot = "http://www.nexus.example.com/repository/my_private_release_repository")
@TestPropertySource("classpath:application-test.properties")

application-test.properties:

thirdpartyService.url=http://localhost:${stubrunner.runningstubs.some-service.port}

ThirdPartyClient.java:

public interface ThirdPartyClient {

    @GetMapping(path = "/third-party-resource")
    ResponseEntity<MyResponse> find();
}

The above fails because ${stubrunner.runningstubs.some-service.port} is not populated in time.

I understand "@AutoConfigureWireMock has nothing to do with @AutoConfiureStubRunner", but the point I was trying to make is that their functionality seems to overlap, maybe unnecessarily.

@marcingrzejszczak
Copy link
Contributor

The functionality doesn't overlap really.

So what you're showing is a known issue and I'll try to resolve it soonish.

@lukasz-gryzbon
Copy link

Thank you. That would be hugly appreciated.

@marcingrzejszczak
Copy link
Contributor

@lukasz-gryzbon you can check this out spring-cloud/spring-cloud-openfeign#455 to see how this could be fixed . After building it locally feign stuff gets registered later thus the property gets properly resolved

@lukasz-gryzbon
Copy link

Thanks @marcingrzejszczak for such a quick reaction.

I have tried to test it, but I must be doing something wrong... I keep getting the old version resolved in Gradle.

Any tips on how to test it?

It may be because our projects use:

    dependencyManagement {
        imports {
            mavenBom 'org.springframework.cloud:spring-cloud-dependencies:Hoxton.SR9'

I just don't know how to include the version I built locally.... :(

@marcingrzejszczak
Copy link
Contributor

marcingrzejszczak commented Jan 12, 2021

You'd have to point Gradle to cloud mavenBom 2020.0.0-SNAPSHOT cause the pr is against the master branch of openfeign. So locally you'd have to install feign stuff from my branch. Then you'd have to point in your gradle build to use the mavenLocal repository. Then you have to keep your fingers crossed that gradle resolves it correctly. Remember to also bump boot to 2.4.1.

@lukasz-gryzbon
Copy link

Yes! It worked!

After several hours of attempts, tweaking and changing approaches, I managed to run the tests against the lazy_bean_registration branch of Open Feign. They passed so the value is available now before the Feign client is created.

Many thanks!

@marcingrzejszczak
Copy link
Contributor

Fixed via spring-cloud/spring-cloud-openfeign#455

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants