-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Adjust keepAlive for test under JDK 9 [ci: last-only] #10732
Conversation
436d7a7
to
e9f8ec6
Compare
@lrytz this is quite tiresome. My review is that it should skip the test on JDK 8, but the twist is that the offending sub-test should be split into a separate test file with |
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 is quite tiresome
I bet..!
LGTM in general.
new ExecutionContextExecutorService { | ||
// Members declared in scala.concurrent.ExecutionContext | ||
def reportFailure(cause: Throwable): Unit = report(null, cause) | ||
|
||
// Members declared in java.util.concurrent.Executor | ||
def execute(r: Runnable): Unit = fjp.execute(r) | ||
|
||
// Members declared in java.util.concurrent.ExecutorService | ||
def awaitTermination(x$1: Long, x$2: java.util.concurrent.TimeUnit): Boolean = ??? | ||
def invokeAll[T](x$1: java.util.Collection[_ <: java.util.concurrent.Callable[T]], x$2: Long, x$3: java.util.concurrent.TimeUnit): java.util.List[java.util.concurrent.Future[T]] = ??? | ||
def invokeAll[T](x$1: java.util.Collection[_ <: java.util.concurrent.Callable[T]]): java.util.List[java.util.concurrent.Future[T]] = ??? | ||
def invokeAny[T](x$1: java.util.Collection[_ <: java.util.concurrent.Callable[T]], x$2: Long, x$3: java.util.concurrent.TimeUnit): T = ??? | ||
def invokeAny[T](x$1: java.util.Collection[_ <: java.util.concurrent.Callable[T]]): T = ??? | ||
def isShutdown(): Boolean = fjp.isShutdown | ||
def isTerminated(): Boolean = fjp.isTerminated | ||
def shutdown(): Unit = fjp.shutdown() | ||
def shutdownNow(): java.util.List[Runnable] = fjp.shutdownNow() | ||
def submit(r: Runnable): java.util.concurrent.Future[_] = fjp.submit(r) | ||
def submit[T](task: Runnable, res: T): java.util.concurrent.Future[T] = fjp.submit(task, res) | ||
def submit[T](task: java.util.concurrent.Callable[T]): java.util.concurrent.Future[T] = fjp.submit(task) | ||
} |
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.
Does ExecutionContext.fromExecutorService(fjp, report(null, _))
not work as expected here?
78ff43a
to
5986bc0
Compare
the previous failure was spurious, so rebased. |
I wonder if the spurious errors are due to using a spuriously ancient JDK.
|
5986bc0
to
95d1211
Compare
This might also be intermittent
|
/rebuild |
Yes, |
it's already known, infamously, as "the less famous flaky test". |
I needed this today. I ran the test suite, just to make sure my computer fans still work (and check the check files), and the fans went silent but the suite was hung. Of course it was this problem, just waiting to time out. |
How about in particular? I've considered that a PR should come with a "motivation", "how I fixed this", etc. |
You were clearly motivated and fixed it very well :) |
Fixes scala/scala-dev#864
The test waits for the pool thread to terminate instead of interrupting it, probably to ensure that the thread wasn't planning on issuing spurious reports of exceptional circumstances.
The existing test used the default FJP available under JDK 8, which has a default keep-alive of 60 seconds, or one year in DX time.
This commit
reflectivelycreates a FJP under JDK 9+ with a short keep-alive, or uses a simple thread pool executor otherwise.