Hi Max,
Here are some comments/questions:
General question, some of the tests use same name for keystore file,
i.e. ks. Will these tests interfere with each other at runtime? The
shell scripts often do some cleanup before starting the real test. I
assume we don't have to concern ourselves with this anymore?
<i18n.java> what does this test trying to test? I don't quite understand
its value... Do we still need it?
<StandardAlgName.java> line 41 uses 1024 as key size, but shell script
has 2048. We should check the three commands completed successfully as well?
<AlgOptions.java> shell script has additional "-sigalg SHA512withRSA"
options for the last test.
<CloneKeyAskPassword.java> line 67, 68, check for non-null return value?
<PassType.java> nit: 2nd copyright year should be 2019,
<SameName.java> line 47 check for 0 exit value?
<PercentSign.java> line 27: typo, should be "containing"
<EC.java> line 48: check for 0 exit value?
<ConciseJarsigner.java> line 216 seems redundant as it should be covered
by line 220
Nice to have java files instead of scripts. :)
Valerie
On 3/4/2019 4:26 PM, Weijun Wang wrote:
Webrev updated at
https://cr.openjdk.java.net/~weijun/8180573/webrev.01
BTW, last time I mistakenly removed ExportPrivateKeyNoPwd.java which is used by
ListKeychainStore.sh. It's now back.
Thanks,
Max
On Feb 15, 2019, at 9:31 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
Hi Philipp,
In most cases, it's just about creating a non-empty file; in some case, the content is
relevant. For the former, I will change it to something like "new byte[10]";
for the latter, I'll use getBytes(UTF_8).
Thanks,
Max
On Feb 15, 2019, at 5:34 PM, Philipp Kunz <philipp.k...@paratix.ch> wrote:
Hi Max,
I don't know if it is important enough, certainly not a serious issue.
In your patch, for example in DiffEnd.java and a few other tests, Strings are
encoded to byte streams with String.getBytes() which uses the default platform
character set to encode the strings.
Manifests, however, always use UTF-8 as the character set to encode. In my
opinion, getBytes(java.nio.charset.StandardCharsets.UTF_8) would be appropriate
to specify the encoding in a platform-independent way.
Now the manifests used in the tests use so few different characters that this
might not even make a real difference because the first around 100 characters
or so of most character sets are the same in most encodings.
I suspect that the encoding might also have been platform-dependent in at least
some of the previous shell tests and therefore this aspect might as well be
addressed separately and is not strictly necessary to just properly convert
shell tests to java.
Regards,
Philipp
On Wed, 2019-02-13 at 11:01 +0800, Weijun Wang wrote:
Please review the fix at
https://cr.openjdk.java.net/~weijun/8180573/webrev.00/
Notes:
- Most changes are just .sh -> .java
- StorePasswordsByShell.sh combined into StorePasswords.java
- In most cases, JarUtils is called to create a jar file. Sometimes the jar
command is called because of delicate differences, for example, jar adds
directory entries also.
- The i18n tests are completely manual described in i18n.html. Old i18n.java is
useless, now is also empty.
- Copyright year updated to 2019, @bug unchanged.
Two files not converted yet:
- ./keytool/console.sh
This is a manual test calling old versions of JDK.
- ./keytool/ListKeychainStore.sh
I tried on this one but "security list-keychains -s ..." has no effect on mach5
machines when calling by ProcessTools. No idea why, I've created a separate bug
(JDK-8218886) for it.
Thanks,
Max
