Re: RFR: 8301767: Convert virtual thread tests to JUnit [v4]
On Wed, 8 Feb 2023 12:25:30 GMT, Alan Bateman wrote: >> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Replace assertTrue(x == null) with assertNull(x) Thank you Alan for the changes. The latest version of this PR (commit 86af1887), looks fine to me. - Marked as reviewed by jpai (Reviewer). PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 12:19:02 GMT, Alan Bateman wrote: >> test/jdk/java/lang/Thread/virtual/HoldsLock.java line 131: >> >>> 129: assertEquals(vthread.getClass().getName(), >>> info.getLockInfo().getClassName()); >>> 130: assertTrue(info.getLockInfo().getIdentityHashCode() == >>> System.identityHashCode(vthread)); >>> 131: assertTrue(info.getLockOwnerId() == vthreadId); >> >> Previously, before the change to these lines, in case of failure, the >> `assertEquals` would have printed even the incorrect/unexpected value in the >> log file/exception. Now with the usage of `assertTrue`, that information is >> lost from the failures. Do you think, in the context of this test, it would >> be worth to continue using assertEquals and just switch the param order to >> match junit expectations. > > We will be coming back to this test in the future (part of it is disabled) so > we can re-visit this test at that time. That sounds fine to me. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 12:18:12 GMT, Alan Bateman wrote: > note that it impacts 45+ usages and I had hoped to avoid changing the tests > too much. I hadn't realized this construct was used in multiple other places. I see that you already fixed them in a newer commit in this PR; thank you. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 09:39:21 GMT, Jaikiran Pai wrote: >> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains five commits: >> >> - Merge >> - Merge >> - Fix typos in comments >> - GetStackTrace.java test missing @requires vm.continuations >> - Initial commit > > test/jdk/java/lang/Thread/virtual/HoldsLock.java line 131: > >> 129: assertEquals(vthread.getClass().getName(), >> info.getLockInfo().getClassName()); >> 130: assertTrue(info.getLockInfo().getIdentityHashCode() == >> System.identityHashCode(vthread)); >> 131: assertTrue(info.getLockOwnerId() == vthreadId); > > Previously, before the change to these lines, in case of failure, the > `assertEquals` would have printed even the incorrect/unexpected value in the > log file/exception. Now with the usage of `assertTrue`, that information is > lost from the failures. Do you think, in the context of this test, it would > be worth to continue using assertEquals and just switch the param order to > match junit expectations. We will be coming back to this test in the future (part of it is disabled) so we can re-visit this test at that time. > Perhaps `assertNull(name.get())`?. Same on line 325. Okay, we can do that, note that it impacts 45+ usages and I had hoped to avoid changing the tests too much. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v4]
> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. > We'd like to convert these JUnit in the main line in advance of other updates > to these tests in 21. The changes are mostly mechanical and trivial: > > - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods > - Tests using data providers are changed to parameterized tests > - The order of the parameters to assertEquals are swapped so that the > expected result is the first parameter > - Usages of expectThrows are changed to assertThrows > - Tests that threw SkipException are changed to the Assumptions API > > There are a small number of drive-by changes to the tests, nothing > significant, e.g. > > - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to > "@run main" as they aren't TestNG tests. > - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to > use a CountDownLatch rather than sleeping, as the original tests weren't very > robust. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Replace assertTrue(x == null) with assertNull(x) - Changes: - all: https://git.openjdk.org/jdk/pull/12426/files - new: https://git.openjdk.org/jdk/pull/12426/files/75176342..86af1887 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12426=03 - incr: https://webrevs.openjdk.org/?repo=jdk=12426=02-03 Stats: 47 lines in 6 files changed: 1 ins; 0 del; 46 mod Patch: https://git.openjdk.org/jdk/pull/12426.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12426/head:pull/12426 PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 09:47:00 GMT, Alan Bateman wrote: > as the alternative implementation of virtual threads (used on x86_32 and a > few other ports) don't allow custom schedulers. Thank you for that detail. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 10:37:16 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains five commits:
>>
>> - Merge
>> - Merge
>> - Fix typos in comments
>> - GetStackTrace.java test missing @requires vm.continuations
>> - Initial commit
>
> test/jdk/jdk/internal/misc/ThreadFlock/WithScopedValue.java line 48:
>
>> 46:
>> 47: private static Stream factories() {
>> 48: return Stream.of(Thread.ofPlatform().factory(),
>> Thread.ofVirtual().factory());
>
> Previously, before this change, the data provided to the test methods,
> comprised of a virtual ThreadFactory and a
> `Executors.defaultThreadFactory()`. Is this switch to using a
> `Thread.ofPlatform().factory()` OK in the context of this test?
The tests run with both platform and virtual threads, it doesn't matter if the
ThreadFactory for platform threads is Thread.ofPlatform().factory() or
Executors.defaultThreadFactory(). It is changed to be former so it's consistent
with the other tests.
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote:
>> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests.
>> We'd like to convert these JUnit in the main line in advance of other
>> updates to these tests in 21. The changes are mostly mechanical and trivial:
>>
>> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods
>> - Tests using data providers are changed to parameterized tests
>> - The order of the parameters to assertEquals are swapped so that the
>> expected result is the first parameter
>> - Usages of expectThrows are changed to assertThrows
>> - Tests that threw SkipException are changed to the Assumptions API
>>
>> There are a small number of drive-by changes to the tests, nothing
>> significant, e.g.
>>
>> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to
>> "@run main" as they aren't TestNG tests.
>> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to
>> use a CountDownLatch rather than sleeping, as the original tests weren't
>> very robust.
>
> Alan Bateman has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains five commits:
>
> - Merge
> - Merge
> - Fix typos in comments
> - GetStackTrace.java test missing @requires vm.continuations
> - Initial commit
test/jdk/jdk/internal/misc/ThreadFlock/WithScopedValue.java line 48:
> 46:
> 47: private static Stream factories() {
> 48: return Stream.of(Thread.ofPlatform().factory(),
> Thread.ofVirtual().factory());
Previously, before this change, the data provided to the test methods,
comprised of a virtual ThreadFactory and a `Executors.defaultThreadFactory()`.
Is this switch to using a `Thread.ofPlatform().factory()` OK in the context of
this test?
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote: >> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge > - Merge > - Fix typos in comments > - GetStackTrace.java test missing @requires vm.continuations > - Initial commit test/jdk/jdk/internal/misc/ThreadFlock/ThreadFlockTest.java line 92: > 90: assertTrue(flock.name() == null); > 91: flock.close(); > 92: assertTrue(flock.name() == null); // after close assertNull, here and line 90? - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v3]
> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. > We'd like to convert these JUnit in the main line in advance of other updates > to these tests in 21. The changes are mostly mechanical and trivial: > > - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods > - Tests using data providers are changed to parameterized tests > - The order of the parameters to assertEquals are swapped so that the > expected result is the first parameter > - Usages of expectThrows are changed to assertThrows > - Tests that threw SkipException are changed to the Assumptions API > > There are a small number of drive-by changes to the tests, nothing > significant, e.g. > > - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to > "@run main" as they aren't TestNG tests. > - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to > use a CountDownLatch rather than sleeping, as the original tests weren't very > robust. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Fix typo in assumption message - Changes: - all: https://git.openjdk.org/jdk/pull/12426/files - new: https://git.openjdk.org/jdk/pull/12426/files/7dc07c4b..75176342 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12426=02 - incr: https://webrevs.openjdk.org/?repo=jdk=12426=01-02 Stats: 6 lines in 3 files changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/12426.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12426/head:pull/12426 PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote:
>> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests.
>> We'd like to convert these JUnit in the main line in advance of other
>> updates to these tests in 21. The changes are mostly mechanical and trivial:
>>
>> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods
>> - Tests using data providers are changed to parameterized tests
>> - The order of the parameters to assertEquals are swapped so that the
>> expected result is the first parameter
>> - Usages of expectThrows are changed to assertThrows
>> - Tests that threw SkipException are changed to the Assumptions API
>>
>> There are a small number of drive-by changes to the tests, nothing
>> significant, e.g.
>>
>> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to
>> "@run main" as they aren't TestNG tests.
>> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to
>> use a CountDownLatch rather than sleeping, as the original tests weren't
>> very robust.
>
> Alan Bateman has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains five commits:
>
> - Merge
> - Merge
> - Fix typos in comments
> - GetStackTrace.java test missing @requires vm.continuations
> - Initial commit
test/jdk/jdk/incubator/concurrent/ScopedValue/ScopeValueAPI.java line 310:
> 308: ScopedValue.where(name, null, () -> {
> 309: assertTrue(name.isBound());
> 310: assertTrue(name.get() == null);
Perhaps `assertNull(name.get())`?. Same on line 325.
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote:
>> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests.
>> We'd like to convert these JUnit in the main line in advance of other
>> updates to these tests in 21. The changes are mostly mechanical and trivial:
>>
>> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods
>> - Tests using data providers are changed to parameterized tests
>> - The order of the parameters to assertEquals are swapped so that the
>> expected result is the first parameter
>> - Usages of expectThrows are changed to assertThrows
>> - Tests that threw SkipException are changed to the Assumptions API
>>
>> There are a small number of drive-by changes to the tests, nothing
>> significant, e.g.
>>
>> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to
>> "@run main" as they aren't TestNG tests.
>> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to
>> use a CountDownLatch rather than sleeping, as the original tests weren't
>> very robust.
>
> Alan Bateman has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains five commits:
>
> - Merge
> - Merge
> - Fix typos in comments
> - GetStackTrace.java test missing @requires vm.continuations
> - Initial commit
test/jdk/java/lang/Thread/virtual/ThreadAPI.java line 82:
> 80: @BeforeAll
> 81: static void setup() throws Exception {
> 82: ThreadFactory factory = Executors.defaultThreadFactory();
Unlike previously, this change now uses a `ThreadFactory` which creates
non-daemon threads. Is that intentional?
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 09:22:36 GMT, Jaikiran Pai wrote:
>> test/jdk/java/lang/Thread/BuilderTest.java line 294:
>>
>>> 292: void testPriority3() {
>>> 293: Thread currentThread = Thread.currentThread();
>>> 294: assumeFalse(currentThread.isVirtual(), "Main test is a virtual
>>> thread");
>>
>> Hello Alan, not specifically related to the changes in this PR, but should
>> that message have been "Main thread ..." instead of "Main test ..."?
>
> Same comment for test methods in `CustomScheduler`
Well spotted. This was a find+replace change so I didn't notice that the
original exception message had a typo.
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 09:32:49 GMT, Jaikiran Pai wrote: > I wonder if testng (and junit) can be configured to fail the test if no test > methods were present, to flag such mistakes. JUnit's CLI tool has: `--fail-if-no-tests Fail and return exit status code 2 if no tests are found.` We can add such a check into `jtreg`'s JUnit runner at https://github.com/openjdk/jtreg/blob/524161b73dca250639ad3f00a42518e384276906/src/share/classes/com/sun/javatest/regtest/agent/JUnitRunner.java#L149-L151 - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote: >> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge > - Merge > - Fix typos in comments > - GetStackTrace.java test missing @requires vm.continuations > - Initial commit test/jdk/java/lang/Thread/virtual/HoldsLock.java line 131: > 129: assertEquals(vthread.getClass().getName(), > info.getLockInfo().getClassName()); > 130: assertTrue(info.getLockInfo().getIdentityHashCode() == > System.identityHashCode(vthread)); > 131: assertTrue(info.getLockOwnerId() == vthreadId); Previously, before the change to these lines, in case of failure, the `assertEquals` would have printed even the incorrect/unexpected value in the log file/exception. Now with the usage of `assertTrue`, that information is lost from the failures. Do you think, in the context of this test, it would be worth to continue using assertEquals and just switch the param order to match junit expectations. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote: >> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge > - Merge > - Fix typos in comments > - GetStackTrace.java test missing @requires vm.continuations > - Initial commit test/jdk/java/lang/Thread/virtual/GetStackTrace.java line 31: > 29: * @enablePreview > 30: * @modules java.base/java.lang:+open > 31: * @run main GetStackTrace That's interesting - so this test was marked as a testng test previously, but had not test methods in it. I had a look at the test run history of this one and I see that the test execution results in: > java/lang/Thread/virtual/GetStackTrace.java > Total tests run: 0, Passes: 0, Failures: 0, Skips: 0 I wonder if testng (and junit) can be configured to fail the test if no test methods were present, to flag such mistakes. Keeping that aside, the change to make this `@run main` looks fine. As for adding the `@requires vm.continuations`, I'm not familiar with its expectations. Looking at the test case, it just captures stacktraces using `Thread.getStackTrace()` and then just does some basic validation of those stacktrace elements. Do we use this `@requires vm.continuations` even in such tests? - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote: >> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge > - Merge > - Fix typos in comments > - GetStackTrace.java test missing @requires vm.continuations > - Initial commit test/jdk/java/lang/Thread/UncaughtExceptionsTest.java line 30: > 28: import org.junit.jupiter.api.Test; > 29: import org.junit.jupiter.api.BeforeAll; > 30: import org.junit.jupiter.api.AfterAll; It looks like BeforeAll and AfterAll imports are unused in this test. - PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 09:13:37 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains five commits:
>>
>> - Merge
>> - Merge
>> - Fix typos in comments
>> - GetStackTrace.java test missing @requires vm.continuations
>> - Initial commit
>
> test/jdk/java/lang/Thread/BuilderTest.java line 294:
>
>> 292: void testPriority3() {
>> 293: Thread currentThread = Thread.currentThread();
>> 294: assumeFalse(currentThread.isVirtual(), "Main test is a virtual
>> thread");
>
> Hello Alan, not specifically related to the changes in this PR, but should
> that message have been "Main thread ..." instead of "Main test ..."?
Same comment for test methods in `CustomScheduler`
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman wrote:
>> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests.
>> We'd like to convert these JUnit in the main line in advance of other
>> updates to these tests in 21. The changes are mostly mechanical and trivial:
>>
>> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods
>> - Tests using data providers are changed to parameterized tests
>> - The order of the parameters to assertEquals are swapped so that the
>> expected result is the first parameter
>> - Usages of expectThrows are changed to assertThrows
>> - Tests that threw SkipException are changed to the Assumptions API
>>
>> There are a small number of drive-by changes to the tests, nothing
>> significant, e.g.
>>
>> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to
>> "@run main" as they aren't TestNG tests.
>> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to
>> use a CountDownLatch rather than sleeping, as the original tests weren't
>> very robust.
>
> Alan Bateman has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains five commits:
>
> - Merge
> - Merge
> - Fix typos in comments
> - GetStackTrace.java test missing @requires vm.continuations
> - Initial commit
test/jdk/java/lang/Thread/BuilderTest.java line 294:
> 292: void testPriority3() {
> 293: Thread currentThread = Thread.currentThread();
> 294: assumeFalse(currentThread.isVirtual(), "Main test is a virtual
> thread");
Hello Alan, not specifically related to the changes in this PR, but should that
message have been "Main thread ..." instead of "Main test ..."?
-
PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit [v2]
> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. > We'd like to convert these JUnit in the main line in advance of other updates > to these tests in 21. The changes are mostly mechanical and trivial: > > - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods > - Tests using data providers are changed to parameterized tests > - The order of the parameters to assertEquals are swapped so that the > expected result is the first parameter > - Usages of expectThrows are changed to assertThrows > - Tests that threw SkipException are changed to the Assumptions API > > There are a small number of drive-by changes to the tests, nothing > significant, e.g. > > - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to > "@run main" as they aren't TestNG tests. > - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to > use a CountDownLatch rather than sleeping, as the original tests weren't very > robust. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge - Merge - Fix typos in comments - GetStackTrace.java test missing @requires vm.continuations - Initial commit - Changes: https://git.openjdk.org/jdk/pull/12426/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12426=01 Stats: 1415 lines in 34 files changed: 205 ins; 79 del; 1131 mod Patch: https://git.openjdk.org/jdk/pull/12426.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12426/head:pull/12426 PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit
On Sat, 4 Feb 2023 08:59:29 GMT, Alan Bateman wrote: > The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. > We'd like to convert these JUnit in the main line in advance of other updates > to these tests in 21. The changes are mostly mechanical and trivial: > > - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods > - Tests using data providers are changed to parameterized tests > - The order of the parameters to assertEquals are swapped so that the > expected result is the first parameter > - Usages of expectThrows are changed to assertThrows > - Tests that threw SkipException are changed to the Assumptions API > > There are a small number of drive-by changes to the tests, nothing > significant, e.g. > > - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to > "@run main" as they aren't TestNG tests. > - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to > use a CountDownLatch rather than sleeping, as the original tests weren't very > robust. Changes look good Alan - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.org/jdk/pull/12426
Re: RFR: 8301767: Convert virtual thread tests to JUnit
On Sat, 4 Feb 2023 08:59:29 GMT, Alan Bateman wrote: > The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. > We'd like to convert these JUnit in the main line in advance of other updates > to these tests in 21. The changes are mostly mechanical and trivial: > > - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods > - Tests using data providers are changed to parameterized tests > - The order of the parameters to assertEquals are swapped so that the > expected result is the first parameter > - Usages of expectThrows are changed to assertThrows > - Tests that threw SkipException are changed to the Assumptions API > > There are a small number of drive-by changes to the tests, nothing > significant, e.g. > > - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to > "@run main" as they aren't TestNG tests. > - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to > use a CountDownLatch rather than sleeping, as the original tests weren't very > robust. Marked as reviewed by cstein (Committer). - PR: https://git.openjdk.org/jdk/pull/12426
