Clean up test_runner Command subclass hash/equality.

tools/testing/dart/test_runner.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Classes and methods for executing tests.
  *
  * This module includes:
  * - Managing parallel execution of tests, including timeout checks.
  * - Evaluating the output of each test as pass/fail/crash/timeout.
@@ skipping 30 matching lines @@
 const List<String> EXCLUDED_ENVIRONMENT_VARIABLES =
     const ['http_proxy', 'https_proxy', 'no_proxy',
            'HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY'];
 
 
 /** A command executed as a step in a test case. */
 class Command {
   /** A descriptive name for this command. */
   String displayName;
 
-  /** The actual command line that will be executed. */
-  String commandLine;
-
   /** Number of times this command *can* be retried */
   int get maxNumRetries => 2;
 
   /** Reproduction command */
   String get reproductionCommand => null;
 
   // We compute the Command.hashCode lazily and cache it here, since it might
   // be expensive to compute (and hashCode is called often).
   int _cachedHashCode;
 
   Command._(this.displayName);
 
   int get hashCode {
     if (_cachedHashCode == null) {
       var builder = new HashCodeBuilder();
       _buildHashCode(builder);
       _cachedHashCode = builder.value;
     }
     return _cachedHashCode;
   }
 
-  operator ==(other) {
-    if (other is Command) {
-      return identical(this, other) || _equal(other as Command);
-    }
-    return false;
+  operator ==(other) => identical(this, other) ||
+      (runtimeType == other.runtimeType && _equal(other));
+
+  void _buildHashCode(HashCodeBuilder builder) {
+    builder.addJson(displayName);
   }
 
-  void _buildHashCode(HashCodeBuilder builder) {
-    builder.add(commandLine);
-    builder.add(displayName);
-  }
-
-  bool _equal(Command other) {
-    return hashCode == other.hashCode &&
-        commandLine == other.commandLine &&
-        displayName == other.displayName;
-  }
+  bool _equal(Command other) =>
+      hashCode == other.hashCode &&
+      displayName == other.displayName;
 
   String toString() => reproductionCommand;
 
   Future<bool> get outputIsUpToDate => new Future.value(false);
 }
 
 class ProcessCommand extends Command {
   /** Path to the executable of this command. */
   String executable;
 
@@ skipping 14 matching lines @@
     if (io.Platform.operatingSystem == 'windows') {
       // Windows can't handle the first command if it is a .bat file or the like
       // with the slashes going the other direction.
       // NOTE: Issue 1306
       executable = executable.replaceAll('/', '\\');
     }
   }
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(executable);
-    builder.add(workingDirectory);
-    for (var object in arguments) builder.add(object);
-    if (environmentOverrides != null) {
-      for (var key in environmentOverrides.keys) {
-        builder.add(key);
-        builder.add(environmentOverrides[key]);
-      }
-    }
+    builder.addJson(executable);
+    builder.addJson(workingDirectory);
+    builder.addJson(arguments);
+    builder.addJson(environmentOverrides);
   }
 
-  bool _equal(Command other) {
-    if (other is ProcessCommand) {
-      if (!super._equal(other)) return false;
-
-      if (hashCode != other.hashCode ||
-          executable != other.executable ||
-          arguments.length != other.arguments.length) {
-        return false;
-      }
-
-      if (!deepJsonCompare(arguments, other.arguments)) return false;
-      if (workingDirectory != other.workingDirectory) return false;
-      if (!deepJsonCompare(environmentOverrides, other.environmentOverrides)) {
-        return false;
-      }
-
-      return true;
-    }
-    return false;
-  }
+  bool _equal(ProcessCommand other) =>
+      super._equal(other) &&
+      executable == other.executable &&
+      deepJsonCompare(arguments, other.arguments) &&
+      workingDirectory == other.workingDirectory &&
+      deepJsonCompare(environmentOverrides, other.environmentOverrides);
 
   String get reproductionCommand {
     var command = ([executable]..addAll(arguments))
         .map(escapeCommandLineArgument).join(' ');
     if (workingDirectory != null) {
       command = "$command (working directory: $workingDirectory)";
     }
     return command;
   }
 
   Future<bool> get outputIsUpToDate => new Future.value(false);
 }
 
 class CompilationCommand extends ProcessCommand {
-  String _outputFile;
-  bool _neverSkipCompilation;
-  List<Uri> _bootstrapDependencies;
+  final String _outputFile;
+  final bool _neverSkipCompilation;
+  final List<Uri> _bootstrapDependencies;
 
   CompilationCommand._(String displayName,
                        this._outputFile,
                        this._neverSkipCompilation,
-                       List<Uri> bootstrapDependencies,
+                       this._bootstrapDependencies,
                        String executable,
                        List<String> arguments,
                        Map<String, String> environmentOverrides)
-      : super._(displayName, executable, arguments, environmentOverrides) {
-    // We sort here, so we can do a fast hashCode/operator==
-    _bootstrapDependencies = new List.from(bootstrapDependencies);
-    _bootstrapDependencies.sort();
-  }
+      : super._(displayName, executable, arguments, environmentOverrides);
 
   Future<bool> get outputIsUpToDate {
     if (_neverSkipCompilation) return new Future.value(false);
 
     Future<List<Uri>> readDepsFile(String path) {
       var file = new io.File(new Path(path).toNativePath());
       if (!file.existsSync()) {
         return new Future.value(null);
       }
       return file.readAsLines().then((List<String> lines) {
@@ skipping 24 matching lines @@
           }
           return true;
         }
       }
       return false;
     });
   }
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(_outputFile);
-    builder.add(_neverSkipCompilation);
-    for (var uri in _bootstrapDependencies) builder.add(uri);
+    builder.addJson(_outputFile);
+    builder.addJson(_neverSkipCompilation);
+    builder.addJson(_bootstrapDependencies);
   }
 
-  bool _equal(Command other) {
-    if (other is CompilationCommand &&
-        super._equal(other) &&
-        _outputFile == other._outputFile &&
-        _neverSkipCompilation == other._neverSkipCompilation &&
-        _bootstrapDependencies.length == other._bootstrapDependencies.length) {
-      for (var i = 0; i < _bootstrapDependencies.length; i++) {
-        if (_bootstrapDependencies[i] != other._bootstrapDependencies[i]) {
-          return false;
-        }
-      }
-      return true;
-    }
-    return false;
-  }
+  bool _equal(CompilationCommand other) =>
+      super._equal(other) &&
+      _outputFile == other._outputFile &&
+      _neverSkipCompilation == other._neverSkipCompilation &&
+      deepJsonCompare(_bootstrapDependencies, other._bootstrapDependencies);
+}
+
+/// This is just a Pair(String, Map) class with hashCode and operator ==
+class AddFlagsKey {
+  final String flags;
+  final Map env;
+  AddFlagsKey(this.flags, this.env);
+  // Just use object identity for environment map
+  bool operator ==(other) =>
+    other is AddFlagsKey && flags == other.flags && env == other.env;
+  int get hashCode => flags.hashCode ^ env.hashCode;
 }
 
 class ContentShellCommand extends ProcessCommand {
   ContentShellCommand._(String executable,
                         String htmlFile,
                         List<String> options,
                         List<String> dartFlags,
                         Map<String, String> environmentOverrides)
       : super._("content_shell",
                executable,
                _getArguments(options, htmlFile),
                _getEnvironment(environmentOverrides, dartFlags));
 
-  static Map _getEnvironment(Map<String, String> env, List<String> dartFlags) {
+  // Cache the modified environments in a map from the old environment and
+  // the string of Dart flags to the new environment.  Avoid creating new
+  // environment object for each command object.
+  static Map<AddFlagsKey, Map> environments =
+      new Map<AddFlagsKey, Map>();
+
+  static Map _getEnvironment(Map env, List<String> dartFlags) {
     var needDartFlags = dartFlags != null && dartFlags.length > 0;
-
     if (needDartFlags) {
-      if (env != null) {
-        env = new Map<String, String>.from(env);
-      } else {
-        env = new Map<String, String>();
+      if (env == null) {
+        env = const { };
       }
-      env['DART_FLAGS'] = dartFlags.join(" ");
-      env['DART_FORWARDING_PRINT'] = '1';
+      var flags = dartFlags.join(' ');
+      return environments.putIfAbsent(new AddFlagsKey(flags, env),
+          () => new Map.from(env)
+              ..addAll({'DART_FLAGS': flags, 'DART_FORWARDING_PRINT': '1'}));
     }
-
     return env;
   }
 
   static List<String> _getArguments(List<String> options, String htmlFile) {
     var arguments = new List.from(options);
     arguments.add(htmlFile);
     return arguments;
   }
 
-  bool _equal(Command other) {
-    return other is ContentShellCommand && super._equal(other);
-  }
-
   int get maxNumRetries => 3;
 }
 
 class BrowserTestCommand extends Command {
   final String browser;
   final String url;
   final Map configuration;
 
   BrowserTestCommand._(String _browser,
                        this.url,
                        this.configuration)
       : super._(_browser), browser = _browser;
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(browser);
-    builder.add(url);
+    builder.addJson(browser);
+    builder.addJson(url);
     builder.add(configuration);
   }
 
-  bool _equal(Command other) {
-    return
-        other is BrowserTestCommand &&
-        super._equal(other) &&
-        browser == other.browser &&
-        url == other.url &&
-        identical(configuration, other.configuration);
-  }
+  bool _equal(BrowserTestCommand other) =>
+      super._equal(other) &&
+      browser == other.browser &&
+      url == other.url &&
+      identical(configuration, other.configuration);
 
   String get reproductionCommand {
     var parts = [TestUtils.dartTestExecutable.toString(),
                 'tools/testing/dart/launch_browser.dart',
                 browser,
                 url];
     return parts.map(escapeCommandLineArgument).join(' ');
   }
 }
 
 class AnalysisCommand extends ProcessCommand {
   final String flavor;
 
   AnalysisCommand._(this.flavor,
                     String displayName,
                     String executable,
                     List<String> arguments,
                     Map<String, String> environmentOverrides)
       : super._(displayName, executable, arguments, environmentOverrides);
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(flavor);
+    builder.addJson(flavor);
   }
 
-  bool _equal(Command other) {
-    return
-        other is AnalysisCommand &&
-        super._equal(other) &&
-        flavor == other.flavor;
-  }
+  bool _equal(AnalysisCommand other) =>
+      super._equal(other) &&
+      flavor == other.flavor;
 }
 
 class VmCommand extends ProcessCommand {
   VmCommand._(String executable,
               List<String> arguments,
               Map<String,String> environmentOverrides)
       : super._("vm", executable, arguments, environmentOverrides);
 }
 
 class JSCommandlineCommand extends ProcessCommand {
@@ skipping 15 matching lines @@
                String pubspecYamlDirectory,
                String pubCacheDirectory)
       : super._('pub_$pubCommand',
                 new io.File(pubExecutable).absolute.path,
                 [pubCommand],
                 {'PUB_CACHE' : pubCacheDirectory},
                 pubspecYamlDirectory), command = pubCommand;
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(command);
+    builder.addJson(command);
   }
 
-  bool _equal(Command other) {
-    return
-        other is PubCommand &&
-        super._equal(other) &&
-        command == other.command;
-  }
+  bool _equal(PubCommand other) =>
+      super._equal(other) &&
+      command == other.command;
 }
 
 /* [ScriptCommand]s are executed by dart code. */
 abstract class ScriptCommand extends Command {
   ScriptCommand._(String displayName) : super._(displayName);
 
   Future<ScriptCommandOutputImpl> run();
 }
 
 class CleanDirectoryCopyCommand extends ScriptCommand {
@@ skipping 26 matching lines @@
       return new ScriptCommandOutputImpl(
           this, Expectation.PASS, "", watch.elapsed);
     }).catchError((error) {
       return new ScriptCommandOutputImpl(
           this, Expectation.FAIL, "An error occured: $error.", watch.elapsed);
     });
   }
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(_sourceDirectory);
-    builder.add(_destinationDirectory);
+    builder.addJson(_sourceDirectory);
+    builder.addJson(_destinationDirectory);
   }
 
-  bool _equal(Command other) {
-    return
-        other is CleanDirectoryCopyCommand &&
-        super._equal(other) &&
-        _sourceDirectory == other._sourceDirectory &&
-        _destinationDirectory == other._destinationDirectory;
-  }
+  bool _equal(CleanDirectoryCopyCommand other) =>
+      super._equal(other) &&
+      _sourceDirectory == other._sourceDirectory &&
+      _destinationDirectory == other._destinationDirectory;
 }
 
 class ModifyPubspecYamlCommand extends ScriptCommand {
   String _pubspecYamlFile;
   String _destinationFile;
   Map<String, Map> _dependencyOverrides;
 
   ModifyPubspecYamlCommand._(this._pubspecYamlFile,
                              this._destinationFile,
                              this._dependencyOverrides)
@@ skipping 43 matching lines @@
       return new ScriptCommandOutputImpl(
           this, Expectation.PASS, "", watch.elapsed);
     }).catchError((error) {
       return new ScriptCommandOutputImpl(
           this, Expectation.FAIL, "An error occured: $error.", watch.elapsed);
     });
   }
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(_pubspecYamlFile);
-    builder.add(_destinationFile);
+    builder.addJson(_pubspecYamlFile);
+    builder.addJson(_destinationFile);
     builder.addJson(_dependencyOverrides);
   }
 
-  bool _equal(Command other) {
-    return
-        other is ModifyPubspecYamlCommand &&
-        super._equal(other) &&
-        _pubspecYamlFile == other._pubspecYamlFile &&
-        _destinationFile == other._destinationFile &&
-        deepJsonCompare(_dependencyOverrides, other._dependencyOverrides);
-  }
+  bool _equal(ModifyPubspecYamlCommand other) =>
+      super._equal(other) &&
+      _pubspecYamlFile == other._pubspecYamlFile &&
+      _destinationFile == other._destinationFile &&
+      deepJsonCompare(_dependencyOverrides, other._dependencyOverrides);
 }
 
 /*
  * [MakeSymlinkCommand] makes a symbolic link to another directory.
  */
 class MakeSymlinkCommand extends ScriptCommand {
   String _link;
   String _target;
 
   MakeSymlinkCommand._(this._link, this._target) : super._('make_symlink');
@@ skipping 17 matching lines @@
       return new ScriptCommandOutputImpl(
           this, Expectation.PASS, "", watch.elapsed);
     }).catchError((error) {
       return new ScriptCommandOutputImpl(
           this, Expectation.FAIL, "An error occured: $error.", watch.elapsed);
     });
   }
 
   void _buildHashCode(HashCodeBuilder builder) {
     super._buildHashCode(builder);
-    builder.add(_link);
-    builder.add(_target);
+    builder.addJson(_link);
+    builder.addJson(_target);
   }
 
-  bool _equal(Command other) {
-    return
-        other is MakeSymlinkCommand &&
-        super._equal(other) &&
-        _link == other._link &&
-        _target == other._target;
-  }
+  bool _equal(MakeSymlinkCommand other) =>
+      super._equal(other) &&
+      _link == other._link &&
+      _target == other._target;
 }
 
 class CommandBuilder {
   static final CommandBuilder instance = new CommandBuilder._();
 
   bool _cleared = false;
   final _cachedCommands = new Map<Command, Command>();
 
   CommandBuilder._();
 
@@ skipping 2367 matching lines @@
     }
   }
 
   void eventAllTestsDone() {
     for (var listener in _eventListener) {
       listener.allDone();
     }
     _allDone();
   }
 }