<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 2018/9/27 21:20, Weijun Wang wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:C8B6810A-F93D-4771-98B6-13B04EB4B6C4@oracle.com">
      <pre wrap="">Change looks fine.

On PKCS11Test.java:453, you can use Files::newInputStream.</pre>
    </blockquote>
    Just addressed it, please take a look at this updated webrev:<br>
    <a class="moz-txt-link-freetext"
      href="http://cr.openjdk.java.net/%7Ejjiang/8209546/webrev.03/">http://cr.openjdk.java.net/~jjiang/8209546/webrev.03/</a><br>
    It also renames the test in ProblemList.txt accordingly.<br>
    <br>
    Best regards,<br>
    John Jiang<br>
    <blockquote type="cite"
      cite="mid:C8B6810A-F93D-4771-98B6-13B04EB4B6C4@oracle.com">
      <pre wrap="">Thanks
Max

</pre>
      <blockquote type="cite">
        <pre wrap="">On Sep 27, 2018, at 5:34 PM, <a class="moz-txt-link-abbreviated" href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a> wrote:

Hi Max,
Please review this new webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ejjiang/8209546/webrev.02/">http://cr.openjdk.java.net/~jjiang/8209546/webrev.02/</a>
You previous points, except #1, are addressed.

Best regards,
John Jiang

On 2018/9/27 11:18, <a class="moz-txt-link-abbreviated" href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a> wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="">On 2018/9/27 10:34, Weijun Wang wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="">Hi John

1. Please add @bug to all tests.
</pre>
          </blockquote>
          <pre wrap="">Which issue should be linked? JDK-8209546?
I suppose @bug should indicate a product issue here.
At least, JDK-8209546 looks have no much association with this test.

</pre>
          <blockquote type="cite">
            <pre wrap="">2. Are getLibPath() and findLib() in AutoTest.java really necessary? It looks like PKCS11Test::getNSSLibDir is doing something similar.
</pre>
          </blockquote>
          <pre wrap="">I'll modify PKCS11Test.java a bit to help this point.

</pre>
          <blockquote type="cite">
            <pre wrap="">3. Looks like Standard.java is not necessary now. You can just make KeyToolTest.java a @test and add a @run line there, something like

     @run main/othervm/timeout=600 -Dfile KeyToolTest
</pre>
          </blockquote>
          <pre wrap="">Will address.

</pre>
          <blockquote type="cite">
            <pre wrap="">4. Maybe we can rename AutoTest.java to NssTest.java. The old name says nothing.
</pre>
          </blockquote>
          <pre wrap="">Will address.

Best regards,
John Jiang
</pre>
          <blockquote type="cite">
            <pre wrap="">Thanks
Max

</pre>
            <blockquote type="cite">
              <pre wrap="">On Sep 27, 2018, at 9:25 AM, <a class="moz-txt-link-abbreviated" href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a> wrote:

Hi Max,
Please review the updated webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ejjiang/8209546/webrev.01/">http://cr.openjdk.java.net/~jjiang/8209546/webrev.01/</a>
All your comments are addressed, though this test is moved to problem list for windows due to JDK-8204203.

Best regards,
John Jiang
On 2018/9/25 22:30, Weijun Wang wrote:
</pre>
              <blockquote type="cite">
                <pre wrap="">Some questions:

1. Do we still need the OS check on lines 47-49? As long as getLibPath() can return something, does it mean the test should just run? Especially, does the test run on Windows?

2. Is launching a separate process necessary? Can we just call KeyToolTest::main after setting system properties and copying the files.

3. Is it possible to include standard.sh?

Thanks
Max


</pre>
                <blockquote type="cite">
                  <pre wrap="">On Sep 25, 2018, at 6:30 PM, <a class="moz-txt-link-abbreviated" href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
  wrote:

Hi,
JDK-8164639 removed NSS libs from repo, so sun/security/tools/keytool/autotest.sh has to download NSS libs from artifactory on macosx.
This patch also refactors this shell test to a Java test.

Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ejjiang/8209546/webrev.00/">http://cr.openjdk.java.net/~jjiang/8209546/webrev.00/</a>

Issue:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8209546">https://bugs.openjdk.java.net/browse/JDK-8209546</a>


Best regards,
John Jiang


</pre>
                </blockquote>
              </blockquote>
            </blockquote>
          </blockquote>
        </blockquote>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>