On Mon, 25 Oct 2021 22:21:54 GMT, Jakob Cornell <d...@openjdk.java.net> wrote:
>> This has been under discussion on and off for the past month or so on
>> serviceability-dev, and I think a CSR request is required, so this may be a
>> work in progress.
>>
>> Notes on the patch:
>>
>> - The `list` command previously marked a line in each listing with `=>`. In
>> a bare `list` this is the next line up for execution. Previously when
>> requesting a specific location (e.g. `list 5`) the requested line would be
>> marked. With the patch applied, `list` will only ever mark the next line up
>> for execution. This is consistent with the behavior of GDB and PDB (at
>> least).
>> - `EOF` is printed when the repeat setting is on and a bare `list` command
>> follows a listing containing the last source line. This feature is from
>> PDB; it's a somewhat softer message than the one for an explicit `list`
>> request that's out of range.
>> - I don't speak Chinese or Japanese, so I've omitted localizations for the
>> new messages in those locales. However, I updated the help text in both to
>> include the new commands, with the descriptions left empty for now.
>
> Jakob Cornell has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Restore update of copyright messages in resource files
I would suggest either `Failure` or `TestFailure` instead of `RuntimeException`
(and they both inherit from `RuntimeException`). This is what other tests do
instead of throwing `RuntimeException`. From an implementation standpoint the
two are identical. However, what I've noticed is that a lot of tests catch
`Failure`, but not `TestFailure`, and it almost always looks something like:
} catch (Failure e) {
log.complain("TEST FAILED: " + e.getMessage());
success = false;
...
if (!success) {
log.complain("TEST FAILED");
return FAILED;
}
But those are all in jdi and jdwp tests. Jdb tests seem to almost alway use the
`failure()` method, which is the same as the first part of the above code, and
then later `JdbTest.runtTest()` does the 2nd part, so I think using `failure()`
is more consistent. Yes, this does mean the test will continue to run. This
seems to be the way most of these tests work. A failure does not indicate that
the test should be aborted, and it will continue doing more test cases.
Sometimes this is appropriate. I'm sure plenty of times it's not, but I think
erring on the side of continuing to run the test is generally more useful.
The PR for 8276208 should contain the fix for the failures. If you want to also
clean up the AssertionError issue at the same time, that's ok, or I can file a
separate CR for that part if you wish.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5290
