Re: RFR: 8301767: Convert virtual thread tests to JUnit [v4]

2023-02-08 Thread Jaikiran Pai
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 [v4]

2023-02-08 Thread Alan Bateman
> 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