Fix several little buglets with stepping

Fix several little buglets with stepping

No need to halt on single step if there is a breakpoint on the
same location. While looking at the resulting test failures, I
discovered that the step_in_equals test was not at all testing
what it was meant to test. It happened to pass because of a
related bug in the parser. Decided to abandon that test and
instead check that we can break at == even if one of the operands
is null.

Added ability to match line numbers in the debugger tests.

R=regis@google.com

Committed: https://code.google.com/p/dart/source/detail?r=31898

[regis, 2014-01-16 21:05:02]

LGTM

https://codereview.chromium.org/135843003/diff/50001/tests/standalone/debugge...
File tests/standalone/debugger/break_at_equals_test.dart (right):

https://codereview.chromium.org/135843003/diff/50001/tests/standalone/debugge...
tests/standalone/debugger/break_at_equals_test.dart:1: // Copyright (c) 2013,
the Dart project authors.  Please see the AUTHORS file
2014
Happy New Year!

[hausner, 2014-01-16 21:15:02]

https://codereview.chromium.org/135843003/diff/50001/tests/standalone/debugge...
File tests/standalone/debugger/break_at_equals_test.dart (right):

https://codereview.chromium.org/135843003/diff/50001/tests/standalone/debugge...
tests/standalone/debugger/break_at_equals_test.dart:1: // Copyright (c) 2013,
the Dart project authors.  Please see the AUTHORS file
On 2014/01/16 21:05:02, regis wrote:
> 2014
> Happy New Year!

Stickler! ;-)

[hausner, 2014-01-16 21:15:23]

Committed patchset #3 manually as r31898 (presubmit successful).

[Florian Schneider, 2014-01-17 10:18:01]

https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
File tests/standalone/debugger/break_at_equals_test.dart (right):

https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
tests/standalone/debugger/break_at_equals_test.dart:51: Resume()
Why doesn't this new test does test single stepping at all?

I think stepping into/over == operations should be covered.

[hausner, 2014-01-17 17:13:57]

Ideally we'd make sure that we can step into or over ==. The old test which
attempted to do that was a good example how fragile these kinds of tests are
because they depend on the internal implementation of things. The old test was
completely broken and passed even though it was not at all testing what it
intended to test.

[Florian Schneider, 2014-01-20 10:36:53]

On 2014/01/17 17:13:57, hausner wrote:
> Ideally we'd make sure that we can step into or over ==. The old test which
> attempted to do that was a good example how fragile these kinds of tests are
> because they depend on the internal implementation of things. The old test was
> completely broken and passed even though it was not at all testing what it
> intended to test.

How is it completely broken?

[Florian Schneider, 2014-01-20 11:36:27]

Yes, the test relied on a broken implementation which required two steps of
StepInto to step into a == call. It's still a good idea to cover single stepping
properly.

Instead of removing the test try these changes and it will pass just fine.

In the future please cc me on a CL breaks a test that I added.

https://codereview.chromium.org/135843003/diff/130001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/135843003/diff/130001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:2948: const intptr_t token_pos = TokenPos();
Undo this change.

https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
File tests/standalone/debugger/step_in_equals_test.dart (left):

https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
tests/standalone/debugger/step_in_equals_test.dart:45: StepInto(),
Remove one StepInto. The bug that requires 2 steps seems to be fixed by now.

[Florian Schneider, 2014-01-30 11:03:01]

On 2014/01/20 11:36:27, Florian Schneider wrote:
> Yes, the test relied on a broken implementation which required two steps of
> StepInto to step into a == call. It's still a good idea to cover single
stepping
> properly.
> 
> Instead of removing the test try these changes and it will pass just fine.
> 
> In the future please cc me on a CL breaks a test that I added.
> 
> https://codereview.chromium.org/135843003/diff/130001/runtime/vm/parser.cc
> File runtime/vm/parser.cc (right):
> 
>
https://codereview.chromium.org/135843003/diff/130001/runtime/vm/parser.cc#ne...
> runtime/vm/parser.cc:2948: const intptr_t token_pos = TokenPos();
> Undo this change.
> 
>
https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
> File tests/standalone/debugger/step_in_equals_test.dart (left):
> 
>
https://codereview.chromium.org/135843003/diff/130001/tests/standalone/debugg...
> tests/standalone/debugger/step_in_equals_test.dart:45: StepInto(),
> Remove one StepInto. The bug that requires 2 steps seems to be fixed by now.

Ping. Any response?

[hausner, 2014-01-30 20:51:59]

The test was broken in other ways too, not just because it needed two stepInto
commands. IIRC, the test expected some stepping events to happen in main(), when
in fact they happened in the == method. The VM falsely reported the token
position to be 0 instead of the token position of the ==. Position 0 by accident
mapped to main, so it looked like the test did the right thing. It was a false
positive.

I think tests that break so easily may be more maintenance effort than it's
worth. We should be careful not to get to a point where (little) changes to the
VM result in 20% effort for the change, and 80% effort in placating noisy tests.