remove pointless and arbitrary 4 line frame limit
ClosedPublic

Authored by sitter on Mar 17 2020, 12:38 PM.

Details

Summary

... and update a related test case

the original motivation here was garbage _start traces shouldn't be
considered valuable. of course we have way more advanced measurements for
whether a trace is valuable or not. notably the actual rating. so as
long as we have any frame for evaluation that frame's rating is
relevant.
drkonqi is smart enough to ignore lines that do not matter already.
e.g. everything above the crash handler is ignored, equally originator
functions main/start_thread/start/... are ignored. they do not impact
the rating at all, regardless of the amount of frames under review.
as such it is entirely possible to have fewer than 4 frames under review
and that those 4 frames are all we need (e.g. the crash is in main()
directly)

in fact, the very trace sample testing this was proving how the 4 line
limit made no sense. the frame about KCmdLineArgs is the only relevant
and only valuable frame in the sample and it is 100% useful making the
entire trace useful. it is simply a nullptr dereference in the main.

the frame limit is now 1 and the test sample has been replaced with
a real-life sample from https://bugs.kde.org/show_bug.cgi?id=193032

Test Plan

test passes again (was broken because _starts is now actively skipped)

Diff Detail

Repository
R871 DrKonqi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Mar 17 2020, 12:38 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 17 2020, 12:38 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Mar 17 2020, 12:38 PM
sitter updated this revision to Diff 77821.Mar 17 2020, 12:43 PM

update comment as well

bcooksley requested changes to this revision.Mar 17 2020, 4:55 PM
bcooksley added a subscriber: bcooksley.

There should be no further changes to Dr Konqi at this time as it fails to build from source on both FreeBSD and Windows.
Please see the relevant CI jobs for more information - https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/

This revision now requires changes to proceed.Mar 17 2020, 4:55 PM

There should be no further changes to Dr Konqi at this time as it fails to build from source on both FreeBSD and Windows.
Please see the relevant CI jobs for more information - https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/

That should really be fixed by D28123; I see the latest build was ~ 2 hours ago, but it still fails, maybe the CI still doesn't have latest KCrash?

This makes sense; also it fixes a part of the backtraceparsertest unit test, test_bug168000, which currently fails on master (output after export'ing QT_LOGGING_RULES="*drkonqi*=true"):

3: QDEBUG : BacktraceParserTest::btParserUsefulnessTest(test_bug168000) org.kde.drkonqi.parser: Rating: 24 out of 24 Usefulness: Useless
3: QDEBUG : BacktraceParserTest::btParserUsefulnessTest(test_bug168000) org.kde.drkonqi.parser: 90%: 21.6 70%: 16.8 40%: 9.6
3: QDEBUG : BacktraceParserTest::btParserUsefulnessTest(test_bug168000) org.kde.drkonqi.parser: Have seen stack base: false Lines counted: 3
3: FAIL!  : BacktraceParserTest::btParserUsefulnessTest(test_bug168000) Compared values are not the same
3:    Actual   (btUsefulness): "Useless"
3:    Expected (result)      : "MayBeUseful"
3:    Loc: [/home/ahmad/rpmbuild/dev/drkonqi/src/tests/backtraceparsertest/backtraceparsertest.cpp(71)]

the rating is perfect but gets killed by the frame number limit.

apol accepted this revision.Mar 20 2020, 12:43 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Mar 20 2020, 1:10 PM
This revision was automatically updated to reflect the committed changes.