-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow blocking calls in WorkerTask#dispose
#3213
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,26 +74,50 @@ public void shouldDetectBlockingCallsInOperators() { | |
} | ||
|
||
@Test | ||
public void shouldNotReportScheduledFutureTask() { | ||
for (int i = 0; i < 1_000; i++) { | ||
public void shouldNotReportSchedulerScheduledFutureTask() { | ||
for (int i = 0; i < 10_000; i++) { | ||
Scheduler taskScheduler = Schedulers.newSingle("foo"); | ||
try { | ||
Runnable dummyRunnable = () -> { | ||
}; | ||
|
||
for (int j = 0; j < 257; j++) { | ||
taskScheduler.schedule(dummyRunnable, 200, TimeUnit.MILLISECONDS); | ||
taskScheduler.schedule(dummyRunnable, j + 1, TimeUnit.MILLISECONDS); | ||
} | ||
Disposable disposable = taskScheduler.schedule(dummyRunnable, 1, TimeUnit.SECONDS); | ||
|
||
RaceTestUtils.race(disposable::dispose, disposable::dispose); | ||
RaceTestUtils.race(Schedulers.parallel(), disposable::dispose); | ||
} | ||
finally { | ||
taskScheduler.dispose(); | ||
} | ||
} | ||
} | ||
|
||
@Test | ||
public void shouldNotReportWorkerScheduledFutureTask() { | ||
for (int i = 0; i < 10_000; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On my local machine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it does not have to be the same for CI env since it increases the general run time. Thus, let's keep it as it was before. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a follow up to this comment, I created #3216 which would create an opportunity to use a more heavy crunching when run locally and do a quicker run-through via CI. |
||
Scheduler scheduler = Schedulers.newSingle("foo"); | ||
Scheduler.Worker worker = scheduler.createWorker(); | ||
|
||
try { | ||
Runnable dummyRunnable = () -> { | ||
}; | ||
|
||
for (int j = 0; j < 257; j++) { | ||
worker.schedule(dummyRunnable, j + 1, TimeUnit.MILLISECONDS); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the race was between |
||
} | ||
Disposable disposable = worker.schedule(dummyRunnable, 1, TimeUnit.SECONDS); | ||
|
||
RaceTestUtils.race(Schedulers.parallel(), disposable::dispose); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I use the |
||
} | ||
finally { | ||
worker.dispose(); | ||
scheduler.dispose(); | ||
} | ||
} | ||
} | ||
|
||
void expectBlockingCall(String desc, Consumer<CompletableFuture<Object>> callable) { | ||
Assertions | ||
.assertThatThrownBy(() -> { | ||
|
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.
I've adopted this test with learnings from writing the test below.