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