Test ResolutionImpact equivalence.

Test ResolutionImpact equivalence.

R=sigmund@google.com

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

[Johnni Winther, 2016-04-04 13:14:15]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-04-04 22:32:05]

lgtm - just some minor comments regarding code organization

https://codereview.chromium.org/1856713002/diff/20001/pkg/compiler/lib/src/se...
File pkg/compiler/lib/src/serialization/equivalence.dart (right):

https://codereview.chromium.org/1856713002/diff/20001/pkg/compiler/lib/src/se...
pkg/compiler/lib/src/serialization/equivalence.dart:5: /// Functions for
asserting equivalence across serialization.
Since this is mainly used for testing:
- consider moving this to a testing folder. For example:
compiler/lib/src/testing/*, or renaming to testing.dart?
- seems that we always will be using this with the CheckStrategy or did I miss a
place where we use it with a different strategy? If we always use it with
CheckStrategy, we might want to move that here and possibly remove the
test-strategy abstraction?

https://codereview.chromium.org/1856713002/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/serialization_test.dart (right):

https://codereview.chromium.org/1856713002/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/serialization_test.dart:184: bool elementEquivalence(a,
b)) {
rename elementEquivalence. Some ideas:
- areElementsEquivalent(a, b)
- sameElement(a, b)
- checkElementEquivalence(a, b)
- eqElement(a, b)

I kind of like `sameElement`

[Johnni Winther, 2016-04-05 07:27:37]

https://codereview.chromium.org/1856713002/diff/20001/pkg/compiler/lib/src/se...
File pkg/compiler/lib/src/serialization/equivalence.dart (right):

https://codereview.chromium.org/1856713002/diff/20001/pkg/compiler/lib/src/se...
pkg/compiler/lib/src/serialization/equivalence.dart:5: /// Functions for
asserting equivalence across serialization.
On 2016/04/04 22:32:04, Siggi Cherem (dart-lang) wrote:
> Since this is mainly used for testing:
> - consider moving this to a testing folder. For example:
> compiler/lib/src/testing/*, or renaming to testing.dart?
> - seems that we always will be using this with the CheckStrategy or did I miss
a
> place where we use it with a different strategy? If we always use it with
> CheckStrategy, we might want to move that here and possibly remove the
> test-strategy abstraction?

I wan't to use these tests for assertions during development; that's why they're
here and not in tests/compiler/dart2js. For instance (locally) mod the batch
mode to utilize and validate serialized data. Eventually these could be moved to
tests/compiler/dart2js.

The TestStrategy is used in are*Equivalent which is used in the testing of
impacts.

https://codereview.chromium.org/1856713002/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/serialization_test.dart (right):

https://codereview.chromium.org/1856713002/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/serialization_test.dart:184: bool elementEquivalence(a,
b)) {
On 2016/04/04 22:32:04, Siggi Cherem (dart-lang) wrote:
> rename elementEquivalence. Some ideas:
> - areElementsEquivalent(a, b)
> - sameElement(a, b)
> - checkElementEquivalence(a, b)
> - eqElement(a, b)
> 
> I kind of like `sameElement`

Done.

[Johnni Winther, 2016-04-05 07:36:11]

Description was changed from

==========
Test ResolutionImpact equivalence.
==========

to

==========
Test ResolutionImpact equivalence.

R=sigmund@google.com

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

[Johnni Winther, 2016-04-05 07:36:12]

Committed patchset #3 (id:40001) manually as
e914716bb704b4dabc404d6f877f04b96035617f (presubmit successful).