On 3/11/19 3:02 PM, Derek Thomson wrote:
The first that comes to mind is that it doesn't tell you if the string
was expected or not - i just says string found or not found, but not
the setting of the "expected" flag. So it might say "string not found"
but that was fine in the case where the string isn't expected. Until I
realized that, I was interpreting the failure output incorrectly in
many cases - since the test framework outputs all tests' output even
when just one test in a class fails. This explains why I initially
thought so many unrelated tests were failing!
OK, that's worth fixing. I'm generally big on using the words "expected"
or "unexpected" in messages to make it abundantly clear. I think in this
case, you have to read the text in conjunction with the passed/failed
message, which I agree is less than optimal.
In other contexts I've used messages like:
* passed: string found, as expected
* passed: string not found, as expected
* failed: unexpected string found
* failed: expected string not found
The next is making the diffs better, in one case I had to put the
expected and actual into files to find a tiny difference, and mess
around with diff, as I just couldn't see it by eye. This would
actually be impossible in the case of a whitespace difference perhaps.
This might not actually be do-able in all cases - I haven't thought
too much about it yet. I just know that "spotting the difference"
between expected vs actual could get painful.
Yes, I know this pain point. I don't want to go all the way to
including/providing a full diff library, but it might help to identify
the position of the first difference in terms of line and column, and
maybe the different characters. That would certainly be reasonably easy
to do in the context of JavadocTester.
FWIW, I've done the "extract-to-files and use diff/meld" myself at
times. Trailing whitespace at the end of a line can be particularly
annoying!
Despite all this complaining it wasn't that bad though, quite
straightforward - and I really appreciate the level of testing here,
and that it isn't easy to put in place.
The test base has definitely grown over the years; the basic framework
was in place before I joined the team, but the framework has been
rewritten and improved over the years, and the general policy of adding
good tests for features and bug fixes helps. So, your thanks should go
to everyone past and present who has helped here.
I guess you can't create bugs in OpenJDK yet, so I'll file 2 RFEs for
the suggestions you have noted.
-- Jon
I will sync and fix addContent, thanks for the heads-up there.
On Mon, Mar 11, 2019 at 2:53 PM Jonathan Gibbons
<[email protected] <mailto:[email protected]>> wrote:
On 3/11/19 2:48 PM, Derek Thomson wrote:
> Jonathan - I have an update of this fix in
> http://cr.openjdk.java.net/~jcbeyler/8219691/webrev.01/
>
As a trivial matter, you will have a merge conflict when you sync
with
the master repo: addContent just got renamed to just "add".
-- Jon
