jkube
jkube copied to clipboard
PortForwardServiceTest : Remove use of `Thread.sleep`
Component
JKube Kit
Task description
Description
We have this code in PortForwardServiceTest which seems to be forwarding a port inside Kubernetes using Fabric8 Kubernetes Client
https://github.com/eclipse/jkube/blob/3afc7bf1f8dbe4452826c5ec7ec58de5c9408d53/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServiceTest.java#L98-L100
However, I'm not sure why we have Thread.sleep inside the try block after initializing port forward. We don't seem to be testing anything after this wait.
It's better to just remove this unnecessary waiting, asserting whether value returned from forwardPortAsync is not null should suffice.
Expected Behavior
Thread.sleep is removed from PortForwardServiceTest.simpleScenario
Acceptance Criteria
- [ ] Thread.sleep
is removed fromPortForwardServiceTest.simpleScenario` - [ ]
PortForwardServiceTest.simpleScenarioshould still pass after making these changes
Before you start :red_circle:
:point_down: :point_down: :point_down: :point_down: :point_down: :point_down: :point_down: :point_down: :point_down: :point_down::point_down::point_down::point_down::point_down::point_down::point_down: Make sure you read the contributing guide first. Pay special attention to the ECA agreement section and the requirement to sign-off your commit.
How to manually test my changes
Kubernetes
If you don't have a real Kubernetes cluster available (most probably), you can use Minikube or Kind to test with a local cluster.
- Minikube get started guide
- Kind (Requires Docker) quick start
OpenShift
If you don't have a real OpenShift cluster available (most probably), you can use Red Hat's developer Sandbox for Red Hat OpenShift. The only requirement is to have a Red Hat account.
Once you have your Sandbox environment, you'll need to download the oc tool from the cluster console.
(Press the ? icon and from the context menu select Command line tools, you'll be redirected to https://$subdomain.openshiftapps.com/command-lines-tools where you'll be able to download the CLI for your platform)
The sleep is probably there to force the code to pass through the asynchronous block, the test should be refactored to assert something (an open connection, a log message, etc.)
When I was trying this, test was passing even when I removed Thread.sleep ( probably because we are not checking anything after opening port forward)
test was passing because the test is not asserting anything (just like now).