On Wed, 24 Aug 2022 05:03:43 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> test/jdk/com/sun/jdi/CLETest.java line 409:
>>
>>> 407: // Setup all breakpoints
>>> 408: for (MethodBreakpointData bpData : breakpoints) {
>>> 409: Location loc = findMethodLocation(targetClass,
>>> bpData.method,
>>
>> Specify breakpoints by line number in a method is better than just line
>> number, but did you think about mark them with some tag and parse source
>> file to get line numbers? (like in
>> test/jdk/com/sun/jdi/lib/jdb/JdbTest.java, parseBreakpoints method)
>
> It looks like the tests in com/sun/jdi are split between those that extend
> `JdbTest` and those that extend `TestScaffold`. I'm not too sure of the
> rationale for using one over the other, but I ended up with `TestScaffold`
> because of the test I initially used as a template (BreakpointTest.java).
> `parseBreakpoints()` is only supported by `JdbTest`, so I would need to
> convert the test to extend it instead. I have no idea how disruptive this
> might be to the test.
>
> I think the breakpoints I have are fairly safe from code movement and
> modifications. It would require edits within the methods with the
> breakpoints, all of which are very simple and well commented, and there
> really is no reason to ever edit them in the first place. Also there are 8
> other tests that are sensitive to line numbers (search for THIS TEST IS LINE
> NUMBER SENSITIVE), and they are at much greater risk (adding or removing a
> line anywhere before the breakpoint will break the test).
AFAIR JdbTest is a base class to test jdb functionality, I didn't mean to
extend it. But breakpoint parsing code is quite simple, I thought about add
similar method to TestScaffold instead of findMethodLocation.
As I wrote your way with findMethodLocation is much better than just line
numbers (used in "THIS TEST IS LINE NUMBER SENSITIVE" tests), I'm okay with it.
-------------
PR: https://git.openjdk.org/jdk/pull/9840
