RFR: 8320712: Rewrite BadFactoryTest in pure Java [v2]

[Jaikiran Pai]

On Wed, 24 Jan 2024 06:50:36 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please review this PR which rewrites the BadFactoryTest to pure Java/JUnit. The test is currently implemented using a mix of shell script and a Java main method. 
>> 
>> Reviewers may notice the following changes:
>> 
>> - The shell script file `BadFactoryTest.sh` has been retired and jtreg tags moved over to `BadFactoryTest.java`
>> - The main method has been replaced with a JUnit `@Test` method
>> - The service definition file used to be packaged into a jar file, now the source directory is instead directly added to the classpath using `@library /javax/script/JDK_8196959`
>> - The `@summary` tag was updated since the existing summary was slightly confusing
>> - To make jtreg happy, the SecurityManager-enabled invocation now runs with `-Djava.security.manager=allow` instead of just `-Djava.security.manager`
>> - A sanity check was added to verify that `ScriptEngineManager` can actually find and load `BadFactory`. Such a check would have detected the issue which inspired this rewrite, see  #16585.
>> 
>> Verified by running:
>> 
>> 
>> % make test TEST="test/jdk/javax/script/JDK_8196959"
>>   [..]
>>    TEST                                              TOTAL  PASS  FAIL ERROR   
>>    jtreg:test/jdk/javax/script/JDK_8196959               1     1     0     0   
>> 
>> 
>> Note that the original issue JDK-8196959 is not available in JBS, so my understanding of what the original test does is based on code inspection.
>
> Eirik Bjørsnøs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Remove the @library tag, the META-INF service definition file is already on the classpath
>  - Merge branch 'master' into badfactory-java-rewritte
>  - Add the Java rewrite issue to the @bug list
>  - Merge branch 'master' into badfactory-java-rewritte
>    
>    # Conflicts:
>    #	test/jdk/javax/script/JDK_8196959/BadFactoryTest.sh
>  - Move jtreg tags from before to after imports
>  - Merge branch 'master' into badfactory-java-rewritte
>  - Update the @summary to describe the purpose of the test, not the BadFactory
>  - Add comment describing the initialization of ScriptEngineManager
>  - Implement BadFactoryTest in pure Java

Thank you for the update. This test-only change looks OK to me.

Hello Sundar @sundararajana, given your past work on this test, do you have any thoughts on this change?

-------------

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16830#pullrequestreview-1840572685