[13] RFR 8228456: Enhance tests after JDK-8217375

[Weijun Wang]

> On Jul 28, 2019, at 11:28 PM, Philipp Kunz <philipp.kunz at paratix.ch> wrote:
> 
> Hi Max,
> 
> jarsignerProc is definitively a spot where significant amount of time is spent in the test unlike what is before and what is after it. Have you measured how much your change would make the test faster? My bet is it would not be significant. Tell me if I'm wrong.

There is some difference. I first applied your recent patch, then my no-native-library.conf patch and run that test 40 times: mean duration is 153 seconds. Then I apply the change in this mail, it's 59 seconds.

> As I see it, we are, at least to some extent, back to a discussion we already started.

Not exactly. Please note that I haven't called "jarsigner -verify" at all, no matter inside the VM or as a separate process. I simply look into the CodeSigner (and you have already obtained them) to check the number and names.

> ...

> We might consider removing the parallel=true or add a threadPoolSize parameter to it with a suitable value that fits the test machines.

Turns out parallel=true is irrelevant with the failure of this test. It's all about the native library. And after the disabling of native libraries, even running jarsigner as a proc is now acceptable.

This makes the fix of this bug non urgent. I am thinking of fixing it in JDK 14, which means I can include your other src changes.

Thanks,
Max

> 
> Regards,
> Philipp
> 
> 
> 
> On Thu, 2019-07-25 at 17:38 +0800, Weijun Wang wrote:
>> Hi Philipp,
>> 
>> Most are fine but PreserveRawManifestEntryAndDigest.java is still very slow and fails intermittently.
>> 
>> The problem is due to jarsignerProc(). Since you have parallel=true for the test providers, many jarsigner processes would run at the same time and some of our test machines cannot load so many processes (they are also running other jobs).
>> 
>> I read about the place when jarsignerProc() is called and seems you want to make sure the first file is signed by A and the second is not. Can we do this using JarFile API? Does you have other things to check in getExpectedJarSignerOutputUpdatedContentNotValidatedBySignerA()?
>> 
>> How about we remove the `jarsigner -verify` call? Like this:
>> 
>>          // double-check reading the files with a verifying JarFile
>>          try (JarFile jar = new JarFile(jarFilename4, true)) {
>>              if (firstAddedFilename != null) {
>>                  JarEntry je1 = jar.getJarEntry(firstAddedFilename);
>>                  jar.getInputStream(je1).readAllBytes();
>> -                assertTrue(je1.getCodeSigners().length > 0);
>> +                CodeSigner[] ss = je1.getCodeSigners();
>> +                assertTrue(ss.length == 2);
>> +                assertTrue(containsSignerA(ss));
>>              }
>>              if (secondAddedFilename != null) {
>>                  JarEntry je2 = jar.getJarEntry(secondAddedFilename);
>>                  jar.getInputStream(je2).readAllBytes();
>> -                assertTrue(je2.getCodeSigners().length > 0);
>> +                CodeSigner[] ss = je2.getCodeSigners();
>> +                assertTrue(ss.length == 1);
>> +                assertFalse(containsSignerA(ss));
>>              }
>>          }
>>  
>> @@ -392,6 +380,17 @@
>>          }
>>      }
>>  
>> +    private static boolean containsSignerA(CodeSigner[] ss) {
>> +        for (CodeSigner s : ss) {
>> +            X509Certificate x = (X509Certificate)
>> +                    s.getSignerCertPath().getCertificates().get(0);
>> +            if (x.getSubjectX500Principal().toString().equals("CN=A")) {
>> +                return true;
>> +            }
>> +        }
>> +        return false;
>> +    }
>> +
>> 
>> Thanks,
>> Max
>> 
>>> On Jul 22, 2019, at 10:02 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>> 
>>> Please take a review at
>>> 
>>>   http://cr.openjdk.java.net/~weijun/8228456/webrev.00/
>>> 
>>> The change is contributed by Philipp Kunz. Since we are now in RDP 2 and we are not allowed to fix non-test P3 (and lower) bugs. I've removed all src/ changes in Philipp's patch (If I read correctly, is mostly on renaming methods, and brings no behavior change) and made necessary change in test codes to use original method names.
>>> 
>>> I haven't read the changes yet. I only made sure they pass on my own system. Oracle's test farm is in maintenance during the weekend.
>>> 
>>> Thanks,
>>> Max
>>> 
>>