Skip to content

Avoid race in test for firstCompletedOf #9112

Merged
lrytz merged 2 commits intoscala:2.13.xfrom
som-snytt:issue/future-test
Jul 15, 2020
Merged

Avoid race in test for firstCompletedOf #9112
lrytz merged 2 commits intoscala:2.13.xfrom
som-snytt:issue/future-test

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Jul 11, 2020

Observed in travis build:

[error] Test scala.concurrent.FutureTest.bug$divissues$hash10513$u0020firstCompletedOf$u0020must$u0020not$u0020leak$u0020references failed: java.lang.AssertionError: Root Future(<not completed>) held reference java.lang.Object@3d785ff2, took 0.011 sec
[error]     at scala.tools.testkit.AssertUtil$.loop$2(AssertUtil.scala:138)
[error]     at scala.tools.testkit.AssertUtil$.$anonfun$assertNotReachable$5(AssertUtil.scala:148)
[error]     at scala.tools.testkit.AssertUtil$.$anonfun$assertNotReachable$5$adapted(AssertUtil.scala:144)
[error]     at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:553)
[error]     at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:551)
[error]     at scala.collection.AbstractIterable.foreach(Iterable.scala:920)
[error]     at scala.collection.IterableOps$WithFilter.foreach(Iterable.scala:890)

Also restore comment in the test that kind of explains what it's supposed to be testing.

som-snytt added 2 commits July 10, 2020 19:02
@som-snytt
Avoid race in test for firstCompletedOf
2b34e97 
Previous version ought to have awaited the first
completed of, but using the parasitic context
should suffice; we check that with an assertion.
@som-snytt
Exchange stack for Stack in assertNotReachable
cd7ac06
@scala-jenkins scala-jenkins added this to the 2.13.4 milestone Jul 11, 2020
@som-snytt som-snytt requested a review from lrytz July 11, 2020 02:17
@som-snytt
Copy link
Contributor Author

som-snytt commented Jul 11, 2020

FSR it suggests lrytz review this PR.

@som-snytt som-snytt mentioned this pull request Jul 13, 2020
Merged
@som-snytt
Copy link
Contributor Author

som-snytt commented Jul 13, 2020 •
edited
Loading

Todo for jdk 11 apparently, as the warnings promised, or was it the particular jdk I was using:

[error] Test scala.concurrent.FutureTest.bug$divissues$hash10513$u0020firstCompletedOf$u0020must$u0020not$u0020leak$u0020references failed: java.lang.reflect.InaccessibleObjectException: Unable to make field final java.lang.ref.ReferenceQueue jdk.internal.ref.CleanerImpl.queue accessible: module java.base does not "opens jdk.internal.ref" to unnamed module @7e8c329a, took 0.007 sec

@lrytz
Copy link
Member

lrytz commented Jul 13, 2020

Todo

[error] Test scala.concurrent.FutureTest.bug$divissues$hash10513$u0020firstCompletedOf$u0020must$u0020not$u0020leak$u0020references failed: java.lang.reflect.InaccessibleObjectException: Unable to make field final java.lang.ref.ReferenceQueue jdk.internal.ref.CleanerImpl.queue accessible: module java.base does not "opens jdk.internal.ref" to unnamed module @7e8c329a, took 0.007 sec

Is that when running the tests on Java 9+?

@forketyfork forketyfork mentioned this pull request Jul 13, 2020
Merged
@som-snytt
Copy link
Contributor Author

som-snytt commented Jul 13, 2020

For future i.e. current JDKs, it would be nice to avoid the warnings anyway about unopened modules for reflection.

Because running the test suite casually, you see errors or warnings and wonder what happened. That's also true of other extraneous output from tests besides actual warnings; tests should run silently so we don't have to scan the output.

@lrytz lrytz merged commit 9b3de11 into scala:2.13.x Jul 15, 2020
@lrytz lrytz mentioned this pull request Jul 15, 2020
Merged
@som-snytt som-snytt deleted the issue/future-test branch July 15, 2020 14:51
@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lrytz lrytz lrytz approved these changes

Assignees

No one assigned

Labels

internal not resulting in user-visible changes (build changes, tests, internal cleanups)

Projects

None yet

Milestone

2.13.4

Development

Successfully merging this pull request may close these issues.

4 participants

@som-snytt @lrytz @SethTisue @scala-jenkins