Clean up test_runner Command subclass hash/equality.

Clean up test_runner Command subclass hash/equality.

BUG=
R=ricow@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41037

[Bill Hesse, 2014-10-09 12:03:51]

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (left):

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:185: _bootstrapDependencies.sort();
All actual uses have only one entry, and if we make hashCode/== depend on order,
the only failure scenario is if two commands are completely identical except for
the order of this list, then we don't merge them, and there could be a race
condition, if they had the same target.   But how could two compilation commands
with the same target directory be made, that didn't follow the same code path
and have the same value for this.

We could also assert the list is sorted here, if we want to exclude this.  We
definitely don't want to make new copies every time.  We could also just make a
new copy if it isn't sorted, or cache the sorted version, but this seems to be
overkill.

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/utils.dart
File tools/testing/dart/utils.dart (right):

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/utils.dar...
tools/testing/dart/utils.dart:253: void addJson(Object object) {
This class is only used for test_runner.dart hashcode/equality for Command
subclasses, so we want to allow URIs and to hash equal maps identically.

[ricow1, 2014-10-09 12:16:48]

ricow@google.com changed reviewers:
+ kustermann@google.com, ricow@google.com

[ricow1, 2014-10-09 12:16:48]

lgtm, +kustermann who did this originally

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:224: static Map<Map, Map<String, Map>>
_modifiedEnvironments =
you may want to do a utility class for this, my mind at least can't understand
this easily

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:251: bool _equal(Command other) =>
super._equal(other);  // We could omit this.
so why don't we

[Bill Hesse, 2014-10-10 11:59:15]

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:224: static Map<Map, Map<String, Map>>
_modifiedEnvironments =
On 2014/10/09 12:16:48, ricow1 wrote:
> you may want to do a utility class for this, my mind at least can't understand
> this easily

Done.  Use a key pair class, to uncurry the map to a map from a pair to a value,
rather than a map of maps.  Should be simpler.  I could have called the class
just "Pair", but gave it a specific name, and specific types.  We could wrap the
cached and returned environments in an UnmodifiableMapView, but I don't think it
is necessary.

https://codereview.chromium.org/645533002/diff/1/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:251: bool _equal(Command other) =>
super._equal(other);  // We could omit this.
On 2014/10/09 12:16:48, ricow1 wrote:
> so why don't we

Done.

[Bill Hesse, 2014-10-10 12:06:02]

Committed patchset #3 (id:100001) manually as 41037 (presubmit successful).