Fabric8 configmap event reload patch refactor part 3 #1467
Conversation
…nto fabric8-configmap-event-reload-patch-refactor-part-3
| @@ -52,9 +52,8 @@ | |||
| <module>spring-cloud-kubernetes-fabric8-istio-it</module> | |||
| <module>spring-cloud-kubernetes-fabric8-client-discovery</module> | |||
| <module>spring-cloud-kubernetes-fabric8-client-loadbalancer</module> | |||
| <module>spring-cloud-kubernetes-fabric8-client-configmap-polling-reload</module> | |||
There was a problem hiding this comment.
I think this is the easiest way to look at this PR: I drop spring-cloud-kubernetes-fabric8-client-configmap-polling-reload and refactor all of its tests by placing them into spring-cloud-kubernetes-fabric8-client-reload.
Logically, the tests do not change at all, just merge them in a different project and use PATCH like testing for them
| @@ -139,6 +144,10 @@ void testInformFromOneNamespaceEventNotTriggered() { | |||
| testInformFromOneNamespaceEventTriggered(); | |||
| testInform(); | |||
| testInformFromOneNamespaceEventTriggeredSecretsDisabled(); | |||
| testDataChangesInConfigMap(); | |||
There was a problem hiding this comment.
all methods that were in the spring-cloud-kubernetes-fabric8-client-configmap-polling-reload were ported in this test now
| @@ -116,7 +116,7 @@ public static void assertReloadLogStatements(String left, String right, String a | |||
| LOG.info("appPodName : ->" + appPodName + "<-"); | |||
| // we issue a pollDelay to let the logs sync in, otherwise the results are not | |||
| // going to be correctly asserted | |||
| await().pollDelay(20, TimeUnit.SECONDS).pollInterval(Duration.ofSeconds(5)).atMost(Duration.ofSeconds(600)) | |||
There was a problem hiding this comment.
lower the interval from 600 to 120
|
@ryanjbaxter ready to be looked at. thank you |
| Commons.waitForLogStatement("will add file-based property source : /tmp/application.properties", container, | ||
| appLabelValue); | ||
| // (3) | ||
| WebClient webClient = builder().baseUrl("http://localhost/key").build(); |
There was a problem hiding this comment.
Why use WebClient instad of RestClient?
There was a problem hiding this comment.
all of our integration tests are using WebClient... not exactly sure what you are trying to suggest tbh
There was a problem hiding this comment.
Huh, I guess I never noticed. It just seemed odd to me to use WebClient and then block when you could just use RestTemplate
There was a problem hiding this comment.
huh! good point :) I think that initially we used it because of RestTemplate deprecation, but now that you put it like this, I can't recall the exact history behind it
There was a problem hiding this comment.
RestTemplate isn't going anywhere. Anyways it is no big deal, its a test
| @@ -26,9 +26,12 @@ | |||
| <artifactId>spring-boot-starter-actuator</artifactId> | |||
| </dependency> | |||
|
|
|||
| <!-- not bootstrap starter, because we want to be able to disable bootstrap per-test --> | |||
There was a problem hiding this comment.
Where are the tests that are not using bootstrap?
There was a problem hiding this comment.
ConfigMapPollingReloadDelegate.testConfigMapPollingReload
and
ConfigMapMountPollingReloadDelegate.testConfigMapMountPollingReload
The way to find out this info after our refactoring is to look where the body is for the PATCH and there will be an environment variable in the form:
{
"name": "SPRING_CLOUD_BOOTSTRAP_ENABLED",
"value": "FALSE"
}
There was a problem hiding this comment.
Now I see things clearer. For some reason before I was just looking at a single commit and not all the changes. Sorry for the dumb question.
There was a problem hiding this comment.
ah, no! every question we discuss closes a few gaps here and there.
No description provided.