" LiveReferencesTest you wrote looks good to me on first glance. " - ok so I created a PR https://github.com/apache/netbeans/pull/5417
"By unused code you mean the "fromRoots" variants in LiveReferences which aren't used as you pointed out? Although this would be an interesting way to increase test coverage I wouldn't do that no. There is no harm in leaving them there - they do also sound useful to me." There has been a misunderstanding. My intention is not to fiddle with LiveReferences class at all. Please look through coverage report - https://drive.google.com/file/d/1-IdW4Ci-wMEsh2rKm9KbsXRbAxlo-eCA/view?usp=share_link My intention is to remove all the code which is unreachable (colored red) when invoking LiveReferences.fromRoots methods as it is probably a dead code. On Thu, Feb 2, 2023 at 10:07 PM Michael Bien <[email protected]> wrote: > On 02.02.23 18:35, Łukasz Bownik wrote: > > Hello. > > In the past I submitter a cleanup PR concerning > https://github.com/apache/netbeans/tree/master/harness/o.n.insane > > I was notified that this modules tests are weak (they are actually smoke > tests which do not verify much) and cover only 22% of code. > > > I cooked closer at module's exposed interface and realized that only 2wo > functions (3-arg and 4-arg ones) from > > https://github.com/apache/netbeans/blob/master/harness/o.n.insane/src/org/netbeans/insane/live/LiveReferences.java > > are used by two other modules > > [image: image.png] > in the following places > > https://github.com/apache/netbeans/blob/acffecd4386a138f0255302b1686111ae445fc2c/harness/nbjunit/src/org/netbeans/junit/NbTestCase.java#L1594 > > https://github.com/apache/netbeans/blob/acffecd4386a138f0255302b1686111ae445fc2c/apisupport/timers/src/org/netbeans/modules/timers/TimeComponentPanel.java#L442 > > I've written some new tests without submitting PR yet > > https://github.com/apache/netbeans/compare/master...lbownik:netbeans:insane.tests > that verifies the behavior of only the 2 udes methods. > > It turns out that the coverage is only 25% and most other the code of this > module is unused (zipped coverage > https://drive.google.com/file/d/1-IdW4Ci-wMEsh2rKm9KbsXRbAxlo-eCA/view?usp=share_link > ). > > If this PR gerts merged, could the unused code be deleted in next PR? > > By unused code you mean the "fromRoots" variants in LiveReferences which > aren't used as you pointed out? > > Although this would be an interesting way to increase test coverage I > wouldn't do that no. There is no harm in leaving them there - they do also > sound useful to me. > > LiveReferencesTest you wrote looks good to me on first glance. > > An alternative approach to this would be to actually test LiveEngine and > the trace method. Since LiveReferences are essentially just trivial utility > methods. If LiveEngine is tested there wouldn't be a need to test > LiveReferences since you don't want to test things twice. > > best regards, > > michael > > > > > > > -- > Łukasz Bownik > > > -- Łukasz Bownik
