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