Differential D14337

Fix GdbTest::testUpdateBreakpoint from messed-up break points positions
ClosedPublic
Actions

Authored by kossebau on Jul 24 2018, 3:33 PM.

Details

Reviewers
mwolff
Group Reviewers
KDevelop
Commits
R32:83ca6cf452ee: Fix GdbTest::testUpdateBreakpoint from messed-up break points positions
Summary

Commit e4b139268ccf9be93b0049c503d4caab6241b4ec added some lines to the
sources of the debugged code, but only updated the first breakpoint
position, with the second manually inserted suddenly being before the
first.
This results in a race condition, given the command for the
second breakpoint is only executed when the actual program is also run
and might have already reached the originally set breakpoint. In that case
the manually set breakpoint no longer will be reached, breaking the
assumptions of the test case.

Remove also extra session->stepInto(); which served no obvious purpose
for the given test case.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Jul 24 2018, 3:33 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJul 24 2018, 3:33 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
kossebau requested review of this revision.Jul 24 2018, 3:33 PM
kossebau added a subscriber: arrowd.Jul 24 2018, 5:00 PM
In R32:e4b139268ccf9be93b0049c503d4caab6241b4ec#3677576, @arrowd wrote:

@kossebau I think it is a nice idea to include the line you intend to break on in the comments. I did the same in https://phabricator.kde.org/R32:6c253502d22a85a135ae131ab86bbae33b4b78e3 and hope this would reduce further breakage.

Yes, considered as well, but for some reason dropped it again (the double foo(); line made this ambiguous and made me unhappy about the approach).

For some moments wondered if I should not add comments with tags to the actual debugged code which then gets parsed to derive the real line number).

Let's see if I get to solve the other tests falling on CI for this test currently tonight (sadly not for me), might give it a try, otherwise just add comments as you proposed.

mwolff accepted this revision.Jul 24 2018, 8:43 PM
mwolff added a subscriber: mwolff.

if it makes tests pass, I'm all for it

This revision is now accepted and ready to land.Jul 24 2018, 8:43 PM
Closed by commit R32:83ca6cf452ee: Fix GdbTest::testUpdateBreakpoint from messed-up break points positions (authored by kossebau). · Explain WhyJul 24 2018, 8:59 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mplugins/gdb/unittests/test_gdb.cpp (19 lines)

Diff 38357

View Options

plugins/gdb/unittests/test_gdb.cpp

Loading...