From 15ee807b85f6b9d8493312a30db2c66cb50038f7 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 7 Apr 2020 13:43:37 +0200 Subject: [PATCH] Refactor tests and make them not rely on `LoadBalancerFeignClient` side effects. --- ...lyCreatedLoadBalancerFeignClientTests.java | 30 +++++++++-------- ...dDelegateLoadBalancerFeignClientTests.java | 32 +++++++++---------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java index 2d60c1d3c..b26ecce6b 100644 --- a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java @@ -59,10 +59,10 @@ import static org.assertj.core.api.BDDAssertions.then; public class ManuallyCreatedLoadBalancerFeignClientTests { @Autowired - MyClient myClient; + MyLoadBalancerClient myLoadBalancerClient; @Autowired - MyNameRemote myNameRemote; + AnnotatedFeignClient annotatedFeignClient; @Autowired ArrayListSpanReporter reporter; @@ -74,29 +74,29 @@ public class ManuallyCreatedLoadBalancerFeignClientTests { @Test public void should_reuse_custom_feign_client() { - String response = this.myNameRemote.get(); + String response = this.annotatedFeignClient.get(); - then(this.myClient.wasCalled()).isTrue(); + then(this.myLoadBalancerClient.wasCalled()).isTrue(); then(response).isEqualTo("foo"); List spans = this.reporter.getSpans(); // retries then(spans).hasSize(1); - then(spans.get(0).tags().get("http.path")).isEqualTo("/"); + then(spans.get(0).tags().get("http.path")).isEqualTo("/test"); } @Test public void my_client_called() { - this.myNameRemote.get(); - then(this.myClient.wasCalled()).isTrue(); + this.annotatedFeignClient.get(); + then(this.myLoadBalancerClient.wasCalled()).isTrue(); } @Test public void span_captured() { - this.myNameRemote.get(); + this.annotatedFeignClient.get(); List spans = this.reporter.getSpans(); // retries then(spans).hasSize(1); - then(spans.get(0).tags().get("http.path")).isEqualTo("/"); + then(spans.get(0).tags().get("http.path")).isEqualTo("/test"); } } @@ -109,7 +109,8 @@ class Application { @Bean public Client client(CachingSpringLoadBalancerFactory cachingFactory, SpringClientFactory clientFactory) { - return new MyClient(new MyDelegateClient(), cachingFactory, clientFactory); + return new MyLoadBalancerClient(new MyDelegateClient(), cachingFactory, + clientFactory); } @Bean @@ -124,9 +125,10 @@ class Application { } -class MyClient extends LoadBalancerFeignClient { +class MyLoadBalancerClient extends LoadBalancerFeignClient { - MyClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory, + MyLoadBalancerClient(Client delegate, + CachingSpringLoadBalancerFactory lbClientFactory, SpringClientFactory clientFactory) { super(delegate, lbClientFactory, clientFactory); } @@ -161,9 +163,9 @@ class MyDelegateClient implements Client { } @FeignClient(name = "foo", url = "http://foo") -interface MyNameRemote { +interface AnnotatedFeignClient { - @RequestMapping(value = "/", method = RequestMethod.GET) + @RequestMapping(value = "/test", method = RequestMethod.GET) String get(); } diff --git a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java index b3a0f8a61..d93971b8d 100644 --- a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java @@ -65,13 +65,13 @@ import static org.assertj.core.api.BDDAssertions.then; public class ManuallyCreatedDelegateLoadBalancerFeignClientTests { @Autowired - MyClient myClient; + MyLoadBalancerClient myLoadBalancerClient; @Autowired MyDelegateClient myDelegateClient; @Autowired - MyNameRemote myNameRemote; + AnnotatedFeignClient annotatedFeignClient; @Autowired ArrayListSpanReporter reporter; @@ -83,31 +83,31 @@ public class ManuallyCreatedDelegateLoadBalancerFeignClientTests { @Test public void should_reuse_custom_feign_client() { - String response = this.myNameRemote.get(); + String response = this.annotatedFeignClient.get(); - then(this.myClient.wasCalled()).isTrue(); + then(this.myLoadBalancerClient.wasCalled()).isTrue(); then(this.myDelegateClient.wasCalled()).isTrue(); then(response).isEqualTo("foo"); List spans = this.reporter.getSpans(); // retries then(spans).hasSize(1); - then(spans.get(0).tags().get("http.path")).isEqualTo("/"); + then(spans.get(0).tags().get("http.path")).isEqualTo("/test"); } @Test public void my_client_called() { - this.myNameRemote.get(); - then(this.myClient.wasCalled()).isTrue(); + this.annotatedFeignClient.get(); + then(this.myLoadBalancerClient.wasCalled()).isTrue(); then(this.myDelegateClient.wasCalled()).isTrue(); } @Test public void span_captured() { - this.myNameRemote.get(); + this.annotatedFeignClient.get(); List spans = this.reporter.getSpans(); // retries then(spans).hasSize(1); - then(spans.get(0).tags().get("http.path")).isEqualTo("/"); + then(spans.get(0).tags().get("http.path")).isEqualTo("/test"); } } @@ -126,15 +126,15 @@ class Application { public Client client(MyDelegateClient myDelegateClient, CachingSpringLoadBalancerFactory cachingFactory, SpringClientFactory clientFactory) { - return new MyClient(myDelegateClient, cachingFactory, clientFactory); + return new MyLoadBalancerClient(myDelegateClient, cachingFactory, clientFactory); } @Bean - public MyNameRemote myNameRemote(Client client, Decoder decoder, Encoder encoder, + public AnnotatedFeignClient myNameRemote(Client client, Decoder decoder, Encoder encoder, Contract contract) { return Feign.builder().client(client).encoder(encoder).decoder(decoder) .contract(contract) - .target(new HardCodedTarget<>(MyNameRemote.class, "foo", "http://foo")); + .target(new HardCodedTarget<>(AnnotatedFeignClient.class, "foo", "http://foo")); } @Bean @@ -149,9 +149,9 @@ class Application { } -class MyClient extends LoadBalancerFeignClient { +class MyLoadBalancerClient extends LoadBalancerFeignClient { - MyClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory, + MyLoadBalancerClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory, SpringClientFactory clientFactory) { super(delegate, lbClientFactory, clientFactory); } @@ -190,9 +190,9 @@ class MyDelegateClient implements Client { } @FeignClient(name = "foo", url = "http://foo") -interface MyNameRemote { +interface AnnotatedFeignClient { - @RequestMapping(value = "/", method = RequestMethod.GET) + @RequestMapping(value = "/test", method = RequestMethod.GET) String get(); }