> On Aug 12, 2015, at 7:55 AM, Kumar Srinivasan <kumar.x.sriniva...@oracle.com>
> wrote:
>
> Henry,
>
> Generally looks good here are some comments, on my initial
> pass, I am not fully done with args.c I will look at this some
> more later today or tomorrow.
>
> 4.)
> expectingNoDash is expectingMain right ? if so I would rename it so.
>
Not really, it is expecting a argument without dash, such as -cp or -classpath.
>
>
> TestHelper.java:
>
> Why add findInOutput method ? this seems to be identical
> to "matches" at line 606, which does exactly that.
>
To return a string, this is used when trying to match a pattern and get
information from that line, I used to get the starting index for an argument in
verifyOptions.
Guess I can remove that now as each use of verifyOptions all starts from 1.
> ArgFileSyntax.java
> Nice, but I think the creation and storage of test case can be simplified,
> basically loadCases(), also please avoid Xbootclasspath. The trouble
> with these tests which operate loops, are very painful to debug
> and isolate a failing case, on the night before GA.
>
I wrote that because I start with testng as a DataProvider, we can change that.
> Can we do something like this.....
>
> Map<String, List<String>> tests = new ArrayList<>();
> tests.put("testing quotes", Array.asList({{...}, {....}});
> tests.add("no recurse expansion", Array.asList({{...}, {....}});
>
> The execute each test, with a description of what the test is doing.
> This way if a test fails its easy to zero in on the failing test.
>
verifyOutput will print out what line is not matching, so it’s pretty easy to
identify which case failed.
Cheers,
Henry
