Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(284)

Issue 11576053: Add basic standalone debugger test (Closed)

Created:
8 years ago by hausner
Modified:
8 years ago
CC:
reviews_dartlang.org, Ivan Posva
Visibility:
Public.

Description

Add basic standalone debugger test Tests the wire protocol API to the VM debugger engine. This is a first, basic test. Additional functionality will be added later to check the expected value of variables, list libraries, scripts, breakpoints etc. Committed: https://code.google.com/p/dart/source/detail?r=16327

Patch Set 1 #

Patch Set 2 : #

Total comments: 16

Patch Set 3 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+565 lines, -0 lines) Patch
A tests/standalone/debugger/basic_debugger_test.dart View 1 chunk +43 lines, -0 lines 0 comments Download
A tests/standalone/debugger/debug_lib.dart View 1 1 chunk +519 lines, -0 lines 4 comments Download
M tests/standalone/standalone.status View 1 2 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
hausner
Mads, if you could please look at the code in debug_lib.dart and tell me whether ...
8 years ago (2012-12-17 19:02:17 UTC) #1
Mads Ager (google)
LGTM for the dart:io part. https://codereview.chromium.org/11576053/diff/3001/tests/standalone/debugger/debug_lib.dart File tests/standalone/debugger/debug_lib.dart (right): https://codereview.chromium.org/11576053/diff/3001/tests/standalone/debugger/debug_lib.dart#newcode79 tests/standalone/debugger/debug_lib.dart:79: int nexting = 0; ...
8 years ago (2012-12-18 06:38:50 UTC) #2
siva
LGTM https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/debugger/basic_debugger_test.dart File tests/standalone/debugger/basic_debugger_test.dart (right): https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/debugger/basic_debugger_test.dart#newcode37 tests/standalone/debugger/basic_debugger_test.dart:37: SetBreakpoint(8), // Set breakpoint a line 15, in ...
8 years ago (2012-12-19 18:28:58 UTC) #3
hausner
Will check updated test in with a subsequent CL. https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/debugger/basic_debugger_test.dart File tests/standalone/debugger/basic_debugger_test.dart (right): https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/debugger/basic_debugger_test.dart#newcode37 tests/standalone/debugger/basic_debugger_test.dart:37: ...
8 years ago (2012-12-19 22:01:51 UTC) #4
Mads Ager (google)
8 years ago (2012-12-20 07:40:51 UTC) #5
Message was sent while issue was closed.
https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/de...
File tests/standalone/debugger/debug_lib.dart (right):

https://chromiumcodereview.appspot.com/11576053/diff/3001/tests/standalone/de...
tests/standalone/debugger/debug_lib.dart:509: process.stdin.close();
On 2012/12/19 22:01:51, hausner wrote:
> On 2012/12/18 06:38:50, Mads Ager wrote:
> > You should drain stdout and stderr here as well to make sure that dart:io
> closes
> > the pipes:
> > 
> > process.stdout.onData = process.stdout.read;
> > process.stderr.onData = process.stderr.read;
> > 
> > In this case it probably doesn't matter much since this script will
terminate
> > and close everything and a couple of pipe leaks will not matter.
> 
> Done. But is this really the right thing to do since I connect a
> StringInputStream to stdout and stderr in the Debugger() constructor...

Urgh, I missed that. No, in that case you shouldn't do anything here. There is
no harm in doing it since the debugger constructor will overwrite the handlers
before you enter the event loop again but it does look weird. Sorry about that!
:-)

Powered by Google App Engine
This is Rietveld 408576698