Generate outline without transformations in patched_sdk, use it for unit tests

Generate outline without transformations in patched_sdk, use it for unit tests

R=paulberry@google.com

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

[Siggi Cherem (dart-lang), 2017-05-11 21:34:57]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2017-05-11 21:36:02]

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

[Paul Berry, 2017-05-11 21:49:14]

lgtm assuming the two comments below are addressed

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:131: return
loadProgramFromBinary(platformUri.toFilePath());
Shoud this be outlineUri?

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:176: var args =
['--platform=$platformDill', generated.path, "Hello, World!"];
Should this be platformUri (rather than platformDill)?

[Siggi Cherem (dart-lang), 2017-05-11 22:06:58]

Thanks Paul, PTAL

I added two status changes that got exposed with this change. It appears
outlines do not include the "external" marker on methods, so that breaks how we
interpret bool.fromEnvironment. We can fix that separately

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:131: return
loadProgramFromBinary(platformUri.toFilePath());
On 2017/05/11 21:49:14, Paul Berry wrote:
> Shoud this be outlineUri?

Yes! Good catch.

I also noticed this soon after I sent this out. I became suspicious all the
sudden when I saw that tests took about the same time as before, sure enough
this explained it :)

Tests now run in half the time as before.

https://codereview.chromium.org/2881603003/diff/20001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:176: var args =
['--platform=$platformDill', generated.path, "Hello, World!"];
On 2017/05/11 21:49:14, Paul Berry wrote:
> Should this be platformUri (rather than platformDill)?

the VM currently only accepts a path and fails when provided with a Uri  (the
error is misleading: it says that the dill file is invalid)

[Paul Berry, 2017-05-11 22:09:40]

lgtm

[Siggi Cherem (dart-lang), 2017-05-11 22:10:41]

Description was changed from

==========
Generate outline without transformations in patched_sdk, use it for unit tests
==========

to

==========
Generate outline without transformations in patched_sdk, use it for unit tests

R=paulberry@google.com

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

[Siggi Cherem (dart-lang), 2017-05-11 22:10:42]

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

[ahe, 2017-05-31 14:04:40]

Since the run step is "async" the test runner doesn't await it. So it shouldn't
do anything asynchronous that can be interleaved with the next test.

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:174: await
context.ensurePlatformUris();
You can't do this here because this step is "async".

[Siggi Cherem (dart-lang), 2017-05-31 15:44:04]

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:174: await
context.ensurePlatformUris();
On 2017/05/31 14:04:40, ahe wrote:
> You can't do this here because this step is "async".

Should this just assert that context.patformUri is not null? (I mean, in
practice this step is done after other steps that do ensure that the platforms
uris are there. So asserting this would be a way to ensure that you put the Run
step after some Setup step.) If not, what's the appropriate way to handle this?

[ahe, 2017-05-31 16:40:19]

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2881603003/diff/40001/pkg/front_end/test/fast...
pkg/front_end/test/fasta/testing/suite.dart:174: await
context.ensurePlatformUris();
On 2017/05/31 15:44:04, Siggi Cherem (dart-lang) wrote:
> On 2017/05/31 14:04:40, ahe wrote:
> > You can't do this here because this step is "async".
> 
> Should this just assert that context.patformUri is not null? (I mean, in
> practice this step is done after other steps that do ensure that the platforms
> uris are there. So asserting this would be a way to ensure that you put the
Run
> step after some Setup step.) If not, what's the appropriate way to handle
this?

I wasn't able to provide you a general solution for this earlier, but now it's
obvious to me :-)

Simply add another step.

But in this particular case, an assertion should be enough.