On Wed, 24 Aug 2022 19:37:11 GMT, Alex Menkov <[email protected]> wrote:
>> 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. Ok. Thanks for the review! ------------- PR: https://git.openjdk.org/jdk/pull/9840
