fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks

fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks

`Set<dynamic>` should not be a subtype of `Set<int>` in strong mode.

R=rnystrom@google.com

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

[Jennifer Messerly, 2017-08-04 23:29:48]

Description was changed from

==========
fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks
==========

to

==========
fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks

`Set<dynamic>` should not be a subtype of `Set<int>` in strong mode.
==========

[Jennifer Messerly, 2017-08-04 23:29:48]

jmesserly@google.com changed reviewers:
+ bkonyi@google.com, rnystrom@google.com

[Jennifer Messerly, 2017-08-04 23:33:11]

Hi could you take a look at this change?

I'm probably missing some status entries, since I'm very new to this. I'm trying
to fix the test to expect the correct strong mode behavior.

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
File tests/corelib_2/corelib_2.status (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
tests/corelib_2/corelib_2.status:168: iterable_to_set_test: RuntimeError #
is-checks do not implement strong mode type system
are there any other permutations I should test?

this test will fail on all Dart 1 platforms at runtime. (even DDC does not
implement it correctly yet, due to
https://github.com/dart-lang/sdk/issues/28988, because DDC's `is` checks attempt
to guard against divergence from the Dart 1 type system)

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
File tests/corelib_2/iterable_to_set_test.dart (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:49: var setCopy = set3.toSet();
the setCopy local in the outer scope has inferred type Set<int>, so this
implicit downcast was failing.

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:51: Expect.isFalse(setCopy is
Set<String>);
these should be false, not true.

[Bob Nystrom, 2017-08-04 23:45:41]

lgtm

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
File tests/corelib_2/corelib_2.status (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
tests/corelib_2/corelib_2.status:168: iterable_to_set_test: RuntimeError #
is-checks do not implement strong mode type system
On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> are there any other permutations I should test?

If you run:

dart tools/migration/run_tests.dart iterable_to_set_test iterable_to_set_test

It will run just this test on a handful of configurations that we find useful
for the migration.

> this test will fail on all Dart 1 platforms at runtime. (even DDC does not
> implement it correctly yet, due to
> https://github.com/dart-lang/sdk/issues/28988, because DDC's `is` checks
attempt
> to guard against divergence from the Dart 1 type system)

That's OK. We want correct tests and correct status files. If the test doesn't
pass, that's the implementer's problem. :)

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
File tests/corelib_2/iterable_to_set_test.dart (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:49: var setCopy = set3.toSet();
On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> the setCopy local in the outer scope has inferred type Set<int>, so this
> implicit downcast was failing.

In other tests where we've had to split a reused variable, we tend to just
rename instead of introducing a block. How about "set3Copy"?

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:51: Expect.isFalse(setCopy is
Set<String>);
On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> these should be false, not true.

Should we add an isTrue check for is Set<dynamic>?

[Bob Nystrom, 2017-08-04 23:47:11]

On 2017/08/04 23:45:41, Bob Nystrom wrote:
> If you run:
> 
> dart tools/migration/run_tests.dart iterable_to_set_test iterable_to_set_test

Er, that should be:

dart tools/migration/bin/run_tests.dart iterable_to_set_test
iterable_to_set_test

[Jennifer Messerly, 2017-08-05 00:05:43]

thank you! fixed those

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
File tests/corelib_2/corelib_2.status (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/corelib_2.s...
tests/corelib_2/corelib_2.status:168: iterable_to_set_test: RuntimeError #
is-checks do not implement strong mode type system
On 2017/08/04 23:45:41, Bob Nystrom wrote:
> On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> > are there any other permutations I should test?
> 
> If you run:
> 
> dart tools/migration/run_tests.dart iterable_to_set_test iterable_to_set_test
> 
> It will run just this test on a handful of configurations that we find useful
> for the migration.
> 
> > this test will fail on all Dart 1 platforms at runtime. (even DDC does not
> > implement it correctly yet, due to
> > https://github.com/dart-lang/sdk/issues/28988, because DDC's `is` checks
> attempt
> > to guard against divergence from the Dart 1 type system)
> 
> That's OK. We want correct tests and correct status files. If the test doesn't
> pass, that's the implementer's problem. :)

thank you! did that & updated

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
File tests/corelib_2/iterable_to_set_test.dart (right):

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:49: var setCopy = set3.toSet();
On 2017/08/04 23:45:41, Bob Nystrom wrote:
> On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> > the setCopy local in the outer scope has inferred type Set<int>, so this
> > implicit downcast was failing.
> 
> In other tests where we've had to split a reused variable, we tend to just
> rename instead of introducing a block. How about "set3Copy"?

Done.

https://codereview.chromium.org/2993743002/diff/1/tests/corelib_2/iterable_to...
tests/corelib_2/iterable_to_set_test.dart:51: Expect.isFalse(setCopy is
Set<String>);
On 2017/08/04 23:45:41, Bob Nystrom wrote:
> On 2017/08/04 23:33:11, Jennifer Messerly wrote:
> > these should be false, not true.
> 
> Should we add an isTrue check for is Set<dynamic>?

Done.

[Jennifer Messerly, 2017-08-05 00:06:20]

Description was changed from

==========
fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks

`Set<dynamic>` should not be a subtype of `Set<int>` in strong mode.
==========

to

==========
fix corelib_2/iterable_to_set_test to reflect strong mode runtime checks

`Set<dynamic>` should not be a subtype of `Set<int>` in strong mode.

R=rnystrom@google.com

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

[Jennifer Messerly, 2017-08-05 00:06:22]

Committed patchset #2 (id:20001) manually as
e1f4de9a6e0ee1a9da99053396eb8dd03c27edd1 (presubmit successful).