RFR: 8319668: Fixup of jar filename typo in BadFactoryTest.sh [v2]

[Gaurav Chaudhari]

On Wed, 22 Nov 2023 11:32:13 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>>> Looks okay. This test is begging to be re-written in Java, maybe some day.
>>> 
>>> I assume the copyright header will be updated before this change is integrated.
>> 
>> Hi @AlanBateman, do I have to update the copyright year to 2023 manually and amend the commit before `/integrate` ?
>
> @Deigue 
> 
> I was able to update `BadFactoryTest.java` to work as a pure Java test by adding the following jtreg header before the imports:
> 
> 
> /*
>  * @test
>  * @bug 8196959
>  * @summary BadFactory that results in NPE being thrown from ScriptEngineManager
>  * @library /javax/script/JDK_8196959
>  * @build BadFactory BadFactoryTest
>  * @run main/othervm BadFactoryTest
>  * @run main/othervm -Djava.security.manager=allow BadFactoryTest
>  */
> 
> 
> I think we should also add a sanity check that the BadFactory ScriptEngineFactory is actually loaded. (This validation would have caught the jar file typo):
> 
> 
> ScriptEngineManager m = new ScriptEngineManager();
> m.getEngineFactories().stream()
>         .filter(c -> c.getClass() == BadFactory.class)
>         .findAny()
>         .orElseThrow(() -> new IllegalStateException("Expected to find BadFactory"));
> 
> 
> Would you like to include these changes in your PR? If not, I'm happy to create a separate PR to convert this test to Java.

@eirbjo Yes, as you noticed, the jar file does matter. And the reason I suspected it wasn't noticed was because it was in the scenario of running without security manager, So may that part of the code wasn't being actively executed.

As for the rewrite, it does look good. But would it make more sense to bring this change as a separate PR having a own openjdk bug issue # designated to reworking of BadFactoryTest.sh for tracking purposes?

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

PR Comment: https://git.openjdk.org/jdk/pull/16585#issuecomment-1823228953