On 03.02.23 12:58, Łukasz Bownik wrote:
" 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.
lets not do that. Only a small part of the live heap analysis library is
exposed via LiveReferences, this doesn't mean it is a good idea to strip
the rest.
The library is also exposed via friend dependencies as you can see in
project.xml, this is certainly not something your coverage tool is
taking into account.
-mbien
On Thu, Feb 2, 2023 at 10:07 PM Michael Bien <mbie...@gmail.com> 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.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
