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.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
