RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()
Mandy Chung
mandy.chung at oracle.com
Tue Jul 12 02:27:27 UTC 2016
> On Jul 12, 2016, at 12:18 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
>
> Hi,
>
> here comes a new version of this change. I've tried to address most of
> the concerns/suggestions:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v1/
>
This version looks okay in general. I suggest to combine the check and Integer.parseInt in a single method (e.g. “numberAt” or some other better method name).
71 if (index - prevIndex > 1 &&
may read better as "index > prevIndex”
The new comment can simply say “This method is used by regression tests” that should do it. I would not mention the test name in the source since refactoring may make it outdated.
In the new test,
92 if (error) {
93 throw new Exception(invalidVersions[i] +
94 " not recognized as invalid by VersionProps.parseVersionNumbers()");
95 }
An alternative to the above check, it can throw an exception immediate after line 82 when parseVersionNumbers.invoke succeeds.
Mandy
More information about the core-libs-dev
mailing list