On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman <al...@openjdk.org> 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

Reply via email to