-
Notifications
You must be signed in to change notification settings - Fork 515
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
fix: replace AssertJ's deprecated asList() DSL method in IngressEnric… #3351
fix: replace AssertJ's deprecated asList() DSL method in IngressEnric… #3351
Conversation
…herBehavioralTest Signed-off-by: heap-s <dqkushbro@gmail.com>
Eclipse JKube CI ReportStarted new GH workflow run for #3351 (2024-08-30T13:28:47Z) ⚙️ JKube E2E Tests (10633337207)
|
@@ -53,7 +55,7 @@ void create_withNoServices_shouldNotCreateIngress() { | |||
// When | |||
new IngressEnricher(context).create(PlatformMode.kubernetes, klb); | |||
// Then | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asList().isEmpty(); | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asInstanceOf(InstanceOfAssertFactories.LIST).isEmpty(); |
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.
assertThat(klb.build()).extracting(KubernetesList::getItems).asInstanceOf(InstanceOfAssertFactories.LIST).isEmpty(); | |
assertThat(klb.build()).extracting(KubernetesList::getItems).asInstanceOf(InstanceOfAssertFactories.list(HasMetadata.class)).isEmpty(); |
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.
Hey, I'm looking at this and I'm seeing that klb.build() IsInstanceOf Service.class. This was an oversight on my part, but is that preferred over HasMetadata.class or am I thinking of asInstanceOf() the wrong way?
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.
Pushing pushing a commit with asInstanceOf(InstanceOfAssertFactories.list(HasMetadata.class)) right now for each of these. If you could kindly still respond to my query above I'd appreciate it.
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.
In this case it doesn't really matter.
The answer would differ depending on what properties were to be extracted later on from the list (in an hypothetical test).
If you were to access spec (ServiceSpec), then you'd probably want Service. If you were to access metadata (ObjectMeta), then either option would be valid.
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.
Thanks for that answer @manusa I figured that, but because from what I've seen before when I was doing a similar issue it used Container.class. Thanks for making this clearer.
@@ -64,7 +66,7 @@ void create_withServicesAndNoExternalUrls_shouldNotCreateIngress() { | |||
// When | |||
new IngressEnricher(context).create(PlatformMode.kubernetes, klb); | |||
// Then | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asList().singleElement() | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asInstanceOf(InstanceOfAssertFactories.LIST).singleElement() |
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.
@@ -112,7 +114,7 @@ void create_withServiceNotExposed_shouldNotCreateIngress() { | |||
// When | |||
new IngressEnricher(context).create(PlatformMode.kubernetes, klb); | |||
// Then | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asList().singleElement() | |||
assertThat(klb.build()).extracting(KubernetesList::getItems).asInstanceOf(InstanceOfAssertFactories.LIST).singleElement() |
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.
@@ -131,7 +133,7 @@ void create_withServices_shouldCreateNetworkingIngress() { | |||
.hasFieldOrPropertyWithValue("metadata.name", "http") | |||
.hasFieldOrPropertyWithValue("spec.defaultBackend.service.name", "http") | |||
.hasFieldOrPropertyWithValue("spec.defaultBackend.service.port.number", 80) | |||
.extracting("spec.rules").asList().isEmpty(); | |||
.extracting("spec.rules").asInstanceOf(InstanceOfAssertFactories.LIST).isEmpty(); |
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.
@@ -150,6 +152,6 @@ void create_withServicesAndTargetExtensions_shouldCreateExtensionsIngress() { | |||
.hasFieldOrPropertyWithValue("metadata.name", "http") | |||
.hasFieldOrPropertyWithValue("spec.backend.serviceName", "http") | |||
.hasFieldOrPropertyWithValue("spec.backend.servicePort.value", 80) | |||
.extracting("spec.rules").asList().isEmpty(); | |||
.extracting("spec.rules").asInstanceOf(InstanceOfAssertFactories.LIST).isEmpty(); |
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.
…es.list(HasMetadata.class) Signed-off-by: heap-s <dqkushbro@gmail.com>
Quality Gate passedIssues Measures |
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.
LGTM, thx!
…herBehavioralTest
Description
Fixes #3334
Type of change
test, version modification, documentation, etc.)
Checklist