fix: removing Thread.sleep() from KubernetesReconcilerCreatorTest

[sarveshkaushal]

Notes

Replaces the fixed Thread.sleep() in KubernetesReconcilerCreatorTest with semaphore based synchronization.

The custom work-queue key function releases a semaphore after adding the generated request to the controller work queue. The test waits for that signal before asserting on the queue.

This is related to issue #1223

Testing

Ran mvn install on the project root.

[sarveshkaushal]

/assign @brendandburns

[brendandburns]

As with the other ones, please try to minimize the diff for easier reviewing.

[sarveshkaushal]

@brendandburns - Formatting changes in this are expected as I have enclosed the code in try block. I have fixed the formatting changes in other PRs.

[brendandburns]

Please don't use globals that aren't constants in the test since it makes it impossible to run tests in parallel in the same JVM.

[brendandburns]

This change looks ok, except for the global Semaphore part, please switch that to be a test local semaphore instead. Also not sure why you added the shutdown (and thus the try/catch) it's not necessarily bad, but I'm not sure its needed.

[sarveshkaushal]

@brendandburns - Thanks for the feedback! You’re right, the static Semaphore was introducing shared global state, which isn't ideal for parallel test execution.
To fix this cleanly, I’ve pivoted to using Awaitility instead of a Semaphore. This eliminates all static/global state. The PR has been updated with this minimal change. Let me know what you think!

[brendandburns]

I think that Awaitility just uses sleep under the hood (I'm not sure how it could do anything other than poll on that condition) so I'm not sure it really fixes this test.

[sarveshkaushal]

I was debating it with Gemini and here is the reasoning:

Awaitility replaces a blind pause with an intelligent evaluate-and-sleep loop.
Instead of waiting for a time, it waits for a state. If your asynchronous task finishes early, Awaitility catches it immediately and moves to the next test. If the task takes longer than usual because the machine is under load, Awaitility patiently waits up to your maximum timeout limit before failing.

[brendandburns]

Awaitlity is basically just while loop with a bunch of small sleeps. imho, using a semaphore or countdown latch is the right way to do this because it waits exactly the right amount of time.

If you really want to go down this route, just use a while loop, let's not bring in an extra library dependency just for this.

Edited, we already have a dependency on that library, so I guess this is ok, I'd still prefer a synchronization primitive.

[brendandburns]

/lgtm
/approve

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, sarveshkaushal

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment