Repository navigation
fix: remove Thread.sleep() from DefaultWorkQueueTest - #4910
sarveshkaushal wants to merge 2 commits into
Conversation
|
/assign @brendandburns |
|
I think there are a lot of whitespace changes that make this hard to review because it makes the diff much bigger than it needs to be. Please minimize the diff and then I can do a real review. |
|
@brendandburns - Thanks for pointing that out. I ran IntelliJ's code formatter on the file, so the formatting in the changed section is formatter-generated rather than intentional whitespace changes. I checked the Git diff as well, and there don't appear to be whitespace-only changes outside the modified test. |
|
The git diff makes this pretty hard to review. With changes like this that modify the thread synchronization stuff and modify a unit test, minimizing the diff really makes a big difference in reviewability. Can you take a stab at trying to minimize, for example remove the changes to import ordering. |
1812765 to
92ad785
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sarveshkaushal The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@brendandburns - I have fixed the formatting changes. |
| DefaultWorkQueue<String> queue = new DefaultWorkQueue<>(); | ||
| final int producerCount = 10; | ||
| final int consumerCount = 5; | ||
| Semaphore itemDequeued = new Semaphore(0); |
There was a problem hiding this comment.
I don't see a sleep in this test and I think the structure of the test is pretty fundamentally changed. Revert changes to this test?
There was a problem hiding this comment.
Agreed. I have undone the changes in this test.
92ad785 to
1f47eac
Compare
1f47eac to
fa8de8d
Compare
Notes
This change removes the Thread.sleep() dependency on the
DefaultWorkQueueTestand replaces it with explicit thread synchronization.. This is related to issue #1223Testing
Ran
mvn installfrom project root and build completed successfully.