Add integration test: hot reload + incremental compiler

Add integration test: hot reload + incremental compiler

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/ebdb0af64f2d1053c352de800916f1ba0895f351

[Siggi Cherem (dart-lang), 2017-06-06 22:20:24]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2017-06-06 22:22:28]

sigmund@google.com changed reviewers:
+ paulberry@google.com, scheglov@google.com

[Siggi Cherem (dart-lang), 2017-06-06 22:22:29]

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/t...
File pkg/front_end/lib/src/testing/hybrid_file_system.dart (right):

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/t...
pkg/front_end/lib/src/testing/hybrid_file_system.dart:7: library
front_end.src.hybrid_file_system;
FYI - this code is the same as before  (moved from kernel-service.dart to this
shared location)

[Siggi Cherem (dart-lang), 2017-06-06 22:44:07]

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
File pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart (right):

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:118: }, skip: true
/* VM crashes on reload */);
Siva: I forgot to mention, to reproduce the failure you need to:
 - change this to false or remove this argument.
 - run

    out/ReleaseX64/dart
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart

[scheglov, 2017-06-07 00:27:23]

lgtm

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
File pkg/front_end/lib/src/vm/reload.dart (right):

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
pkg/front_end/lib/src/vm/reload.dart:39: VmReloader([int port = 8181]) : port =
port;
[this.port = 8181] and you don't need "port = port" then.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
pkg/front_end/lib/src/vm/reload.dart:42: _createPeer() {
Please add the return type.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
pkg/front_end/lib/src/vm/reload.dart:61: if (isolate['name'].contains('\$main'))
{
I that think using r'$main' would show that we ourself use this nice feature of
Dart :-)

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
pkg/front_end/lib/src/vm/reload.dart:71: /// local machine and listening on
[port] for service protocol requests.
Please document and type what is returned.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/v...
pkg/front_end/lib/src/vm/reload.dart:82: disconnect() async {
Please add the return type.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
File pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart (right):

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:52:
startProgram(args) async {
Please type "args".

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:54:
'--enable-vm-service=0', // Note: use 0 to avoid port collsions.
collisions

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:93: await
startProgram(["2"]);
What means ["2"] arguments here?
Just a signal to pass some arguments to the test script in order to avoid
waiting for hot reload?
Would it be better to use a boolean flag instead?

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:132: });
It might be interesting to have a test with more than one reload.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:179: var sink = new
File.fromUri(outputUri).openWrite();
I can do this.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:182: // libraries
instead.
Is it a requirement for any kernel generation or for this test?

Do we expect that any Dart VM installation will have platform.dill and only this
file has to be used as the source of SDK classes?

Do we prohibit presence of dart:xyz libraries in applications?

Or is it just useless because VM will ignore these libraries?

As you understand, it is a very simple change, I just would like to know the
design we are working toward.

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/test/src/...
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart:189:
writeFile(MemoryFileSystem fs, String fileName, String contents) {
void?

[ahe, 2017-06-07 10:28:58]

ahe@google.com changed reviewers:
+ ahe@google.com

[ahe, 2017-06-07 10:28:59]

DBC

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/t...
File pkg/front_end/lib/src/testing/hybrid_file_system.dart (right):

https://codereview.chromium.org/2925953002/diff/20001/pkg/front_end/lib/src/t...
pkg/front_end/lib/src/testing/hybrid_file_system.dart:7: library
front_end.src.hybrid_file_system;
On 2017/06/06 22:22:29, Siggi Cherem (dart-lang) wrote:
> FYI - this code is the same as before  (moved from kernel-service.dart to this
> shared location)

If this is used or intended for anything else but testing, it shouldn't be in
this directory.

[Siggi Cherem (dart-lang), 2017-06-07 19:47:32]

Description was changed from

==========
Add integration test: hot reload + incremental compiler
==========

to

==========
Add integration test: hot reload + incremental compiler

R=scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ebdb0af64f2d1053c352de800916f1ba0895f351
==========

[Siggi Cherem (dart-lang), 2017-06-07 19:47:34]

Committed patchset #2 (id:40001) manually as
ebdb0af64f2d1053c352de800916f1ba0895f351 (presubmit successful).