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

Eirik Bjorsnos duke at openjdk.org
Wed Nov 22 18:31:06 UTC 2023


On Wed, 22 Nov 2023 18:01:21 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> @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?
>
>> 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?
> 
> We have two options:
> 
> - Withdraw this PR, submit a new PR for the rewrite 
> - Repurpose this PR for the rewrite, updating JBS and PR titles accordingly
> 
> In either case, I can help updating/creating JBS isssues as required.
> 
> I have a slight preference for repurposing, but it's up to you. What do you prefer?

> @eirbjo If it makes sense from perspective of commiters/reviewers, I'll be happy to integrate the changes here and tweak so that it looks good.

Reviewer time is a scarce resource. It would be wasteful to spend review cycles on getting a fix of this `.sh` test integrated now and then immediately follow up with a delete in the rewrite PR.

I think we should handle this change in one PR, not two. If you prefer to keep the history of your `.sh` fix documented, we can repurpose this PR, otherwise we start fresh with a new PR for the rewrite.

> I also wonder if we have BadFactoryTest.java instead of BadFactoryTest.sh , will certain references elsewhere need to be adjusted accordingly, in case these jtreg tests are being referenced in a certain way somewhere.

jtreg will pick this up automatically. I did a search of `BadFactoryTest` across the code base, and it is not referenced outside the file itself.

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

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


More information about the core-libs-dev mailing list