[jdk17u-dev] RFR: 8295087: Manual Test to Automated Test Conversion

[Amos Shi]

On Thu, 8 Aug 2024 12:54:45 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> Backport of [JDK-8295087](https://bugs.openjdk.org/browse/JDK-8295087)
>> - This PR contains two commit
>>   - `Commit 1` is generated by git apply from original commit
>>   - `Commit 2` is adding the missing test case `InconsistentEntries.java`, which was originally added by [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779)
>> 
>> Testing
>> - Local: Test passed on `MacOS 14.5` on Apple M1 Max
>>   - `ExtDirs.java`: Test results: passed: 2
>>   - `ExtDirsChange.java`: Test results: passed: 1
>>   - `ExtDirsDefaultPolicy.java`: Test results: passed: 4
>>   - `InconsistentEntries.java`: Test results: passed: 1
>> - Pipeline: 
>>   - Linux, Windows - Passed
>>   - MacOS - Skipped (`This request was automatically failed because there were no enabled runners online to process the request for more than 1 days.`)
>> - Testing Machine: SAP nightlies Passed on `2024-08-07`
>>   - Automated jtreg test: jtreg_jdk_tier1, Started at 2024-08-06 20:35:20+01:00 
>>     - java/lang/ClassLoader/ExtDirs.java: SUCCESSFUL GitHub 📊 - [20:37:45.782 -> 568 msec]
>>   - Automated jtreg test: jtreg_jdk_tier2, Started at 2024-08-06 21:10:46+01:00
>>     - sun/security/provider/PolicyParser/ExtDirsChange.java: SUCCESSFUL GitHub 📊 - [21:41:01.101 -> 364 msec]
>>     - sun/security/provider/PolicyParser/ExtDirsDefaultPolicy.java#id0: SUCCESSFUL GitHub 📊 - [21:41:01.165 -> 39 msec]
>>     - sun/security/provider/PolicyParser/ExtDirsDefaultPolicy.java#id1: SUCCESSFUL GitHub 📊 - [21:41:01.204 -> 309 msec]
>>     - sun/security/provider/PolicyParser/ExtDirsDefaultPolicy.java#id2: SUCCESSFUL GitHub 📊 - [21:41:01.465 -> 309 msec]
>>     - sun/security/provider/PolicyParser/ExtDirsDefaultPolicy.java#id3: SUCCESSFUL GitHub 📊 - [21:41:01.513 -> 362 msec]
>>     - javax/crypto/CryptoPermissions/InconsistentEntries.java: SUCCESSFUL GitHub 📊 - [21:30:11.979 -> 4,379 msec]
>
> You add [test/jdk/javax/crypto/CryptoPermissions/InconsistentEntries.java](https://github.com/openjdk/jdk17u-dev/pull/2774/files#diff-6921fd8e4c83f33bbb681d27e21f3e6d167036940bd09e42567a4039993c5e5f)  with the test,
> 
> But the JBS issue https://bugs.openjdk.org/browse/JDK-8286779  associated with this test is in jdk20 and it seems it was not backported to 17.
> So is it really a good idea to backport the test ? Looks a bit inconsistent to me .

Hi @MBaesken,

The situation: 
In the original commit 
- https://github.com/openjdk/jdk/commit/a3693ccc617d06137a61050b34646e8a90ed3d7e#diff-6921fd8e4c83f33bbb681d27e21f3e6d167036940bd09e42567a4039993c5e5f
- We have the file `test/jdk/javax/crypto/CryptoPermissions/InconsistentEntries.java` modified
  - and this file was added by [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779)
  - via commit https://github.com/openjdk/jdk/commit/8f400b9aab57d0639721add2ba511bfc0459bd89#diff-6921fd8e4c83f33bbb681d27e21f3e6d167036940bd09e42567a4039993c5e5f

In this situation, we have three options here:
- Option 1. In current PR simply ignore the file `InconsistentEntries.java` 
  - the benefit is: the PR looks cleaner
  - the side effect is: later if we need this test case, someone need to apply the `diff` again
- Option 2. Backport the dependency Jira [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779), then backport [JDK-8295087](https://bugs.openjdk.org/browse/JDK-8295087) again
  - the benefit is: both Jira back ports would looks clean
  - the side effect is: the [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779) touches the file `src/java.base/share/classes/javax/crypto/CryptoPolicyParser.java` - and do we want to Backport this code to `jdk17u-dev` ? currently [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779) is not in the scope 
- Option 3. Copy the `InconsistentEntries.java` together with current PR on [JDK-8295087](https://bugs.openjdk.org/browse/JDK-8295087)
  - the benefit is: we get an `de-facto` `clean` backport for [JDK-8295087](https://bugs.openjdk.org/browse/JDK-8295087)

So
- If you agree on this `Option 3`, please revisit and approve this PR
- If you prefer `Option 2`, I will close this PR, and create two PRs, one for [JDK-8286779](https://bugs.openjdk.org/browse/JDK-8286779), after merged will create another PR for [JDK-8295087](https://bugs.openjdk.org/browse/JDK-8295087)
- I do not like `Option 1`, because it brings trouble for later


Thanks
Amos
A Happy Developer

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/2774#issuecomment-2292514886