D28098: remove pointless and arbitrary 4 line frame limit

[Harald Sitter]

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit R871:80efb36bc807: remove pointless and arbitrary 4 line frame 
limit (authored by sitter).

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28098?vs=77821=78092

REVISION DETAIL
  https://phabricator.kde.org/D28098

AFFECTED FILES
  src/parser/backtraceparser.cpp
  src/tests/backtraceparsertest/backtraceparsertest_data/test_bug192412_b

To: sitter, apol, bcooksley
Cc: ahmadsamir, bcooksley, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D28098: remove pointless and arbitrary 4 line frame limit

[Ahmad Samir]

ahmadsamir added a comment.


  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.

REPOSITORY
  R871 DrKonqi

REVISION DETAIL
  https://phabricator.kde.org/D28098

To: sitter, apol, bcooksley
Cc: ahmadsamir, bcooksley, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D28098: remove pointless and arbitrary 4 line frame limit

[Ahmad Samir]

ahmadsamir added a comment.


  In D28098#629338 , @bcooksley 
wrote:
  
  > 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?

REPOSITORY
  R871 DrKonqi

REVISION DETAIL
  https://phabricator.kde.org/D28098

To: sitter, apol, bcooksley
Cc: ahmadsamir, bcooksley, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D28098: remove pointless and arbitrary 4 line frame limit

[Ben Cooksley]

bcooksley requested changes to this revision.
bcooksley added a comment.
This revision now requires changes to proceed.


  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/

REPOSITORY
  R871 DrKonqi

REVISION DETAIL
  https://phabricator.kde.org/D28098

To: sitter, apol, bcooksley
Cc: bcooksley, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28098: remove pointless and arbitrary 4 line frame limit

[Harald Sitter]

sitter updated this revision to Diff 77821.
sitter added a comment.


  update comment as well

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28098?vs=77817=77821

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D28098

AFFECTED FILES
  src/parser/backtraceparser.cpp
  src/tests/backtraceparsertest/backtraceparsertest_data/test_bug192412_b

To: sitter, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28098: remove pointless and arbitrary 4 line frame limit

[Harald Sitter]

sitter created this revision.
sitter added a reviewer: apol.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
sitter requested review of this revision.

REVISION 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)

REPOSITORY
  R871 DrKonqi

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D28098

AFFECTED FILES
  src/parser/backtraceparser.cpp
  src/tests/backtraceparsertest/backtraceparsertest_data/test_bug192412_b

To: sitter, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart