-
Notifications
You must be signed in to change notification settings - Fork 4k
api: Add a Supplier overload to Context #12935
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
275ad7d
55b44a9
f7faff8
6164702
afc76e4
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 |
|---|---|---|
|
|
@@ -49,6 +49,7 @@ | |
| import java.util.concurrent.TimeoutException; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
| import java.util.function.Supplier; | ||
| import java.util.logging.Handler; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.LogRecord; | ||
|
|
@@ -553,6 +554,46 @@ public Object call() { | |
| current.detach(Context.ROOT); | ||
| } | ||
|
|
||
| @Test | ||
| public void testSupply() throws Exception { | ||
| Context base = Context.current().withValue(PET, "cat"); | ||
| Context current = Context.current().withValue(PET, "fish"); | ||
| current.attach(); | ||
|
|
||
| final Object ret = new Object(); | ||
| Supplier<Object> supplier = new Supplier<Object>() { | ||
| @Override | ||
| public Object get() { | ||
| runner.run(); | ||
| return ret; | ||
| } | ||
| }; | ||
|
|
||
| assertSame(ret, base.supply(supplier)); | ||
| assertSame(base, observed); | ||
| assertSame(current, Context.current()); | ||
|
|
||
| assertSame(ret, current.supply(supplier)); | ||
| assertSame(current, observed); | ||
| assertSame(current, Context.current()); | ||
|
|
||
| final TestError err = new TestError(); | ||
| try { | ||
|
Contributor
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.
Contributor
Author
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. If you feel strongly I can, but I was keeping the style of the tests around it.
Contributor
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.
|
||
| base.supply(new Supplier<Object>() { | ||
| @Override | ||
| public Object get() { | ||
| throw err; | ||
| } | ||
| }); | ||
| fail("Excepted exception"); | ||
| } catch (TestError ex) { | ||
| assertSame(err, ex); | ||
| } | ||
| assertSame(current, Context.current()); | ||
|
|
||
| current.detach(Context.ROOT); | ||
| } | ||
|
|
||
| @Test | ||
| public void currentContextExecutor() { | ||
| QueuedExecutor queuedExecutor = new QueuedExecutor(); | ||
|
|
||
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.
This single test is testing multiple different behaviors. We can rewrite it into 3 different behavior driven tests each following the Given-When-Then structure for testing each behavior:
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 think it would be better for the maintainers to take on that task. I wrote the test to match the existing style, and since I don't know your preferences, I think it would be better for you to adjust it the way you want.
Claude should be able to easily adjust and split these tests. How about we continue with the PR as is and the tests can be adjusted after?