From 98a8dd7f74153cc449da8a64214616ed4d81e750 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Wed, 15 Apr 2015 17:10:56 -0700
Subject: [PATCH] Support running tests on Safari.

See #31

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1062683004
---
 CHANGELOG.md                         |   2 +
 README.md                            |   4 +
 lib/src/backend/test_platform.dart   |   6 +-
 lib/src/executable.dart              |   5 +-
 lib/src/runner/browser/safari.dart   |  73 ++++++++++++++
 lib/src/runner/browser/server.dart   |   2 +
 test/runner/browser/runner_test.dart |  12 +++
 test/runner/browser/safari_test.dart | 140 +++++++++++++++++++++++++++
 test/runner/runner_test.dart         |   2 +-
 9 files changed, 243 insertions(+), 3 deletions(-)
 create mode 100644 lib/src/runner/browser/safari.dart
 create mode 100644 test/runner/browser/safari_test.dart

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0ee8c684..ea89557f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@
 
 [timeout]: https://github.com/dart-lang/test/blob/master/README.md#timeouts
 
+* Support running tests on Safari.
+
 * Add a `--version` flag.
 
 ### 0.12.0-beta.7
diff --git a/README.md b/README.md
index 70404d3b..11b5917b 100644
--- a/README.md
+++ b/README.md
@@ -144,6 +144,10 @@ valid identifiers are:
 * `phantomjs`: Whether the test is running on
   [PhantomJS](http://phantomjs.org/).
 
+* `firefox`: Whether the test is running on Mozilla Firefox.
+
+* `safari`: Whether the test is running on Apple Safari.
+
 * `dart-vm`: Whether the test is running on the Dart VM in any context,
   including Dartium. It's identical to `!js`.
 
diff --git a/lib/src/backend/test_platform.dart b/lib/src/backend/test_platform.dart
index f2577d48..d311d692 100644
--- a/lib/src/backend/test_platform.dart
+++ b/lib/src/backend/test_platform.dart
@@ -36,9 +36,13 @@ class TestPlatform {
   static const TestPlatform firefox = const TestPlatform._("Firefox", "firefox",
       isBrowser: true, isJS: true);
 
+  /// Apple Safari.
+  static const TestPlatform safari = const TestPlatform._("Safari", "safari",
+      isBrowser: true, isJS: true);
+
   /// A list of all instances of [TestPlatform].
   static const List<TestPlatform> all =
-      const [vm, dartium, contentShell, chrome, phantomJS, firefox];
+      const [vm, dartium, contentShell, chrome, phantomJS, firefox, safari];
 
   /// Finds a platform by its identifier string.
   ///
diff --git a/lib/src/executable.dart b/lib/src/executable.dart
index e774dc43..843b47fd 100644
--- a/lib/src/executable.dart
+++ b/lib/src/executable.dart
@@ -72,6 +72,9 @@ bool get _usesTransformer {
 }
 
 void main(List<String> args) {
+  var allPlatforms = TestPlatform.all.toList();
+  if (!Platform.isMacOS) allPlatforms.remove(TestPlatform.safari);
+
   _parser.addFlag("help", abbr: "h", negatable: false,
       help: "Shows this usage information.");
   _parser.addFlag("version", negatable: false,
@@ -87,7 +90,7 @@ void main(List<String> args) {
   _parser.addOption("platform",
       abbr: 'p',
       help: 'The platform(s) on which to run the tests.',
-      allowed: TestPlatform.all.map((platform) => platform.identifier).toList(),
+      allowed: allPlatforms.map((platform) => platform.identifier).toList(),
       defaultsTo: 'vm',
       allowMultiple: true);
   _parser.addOption("concurrency",
diff --git a/lib/src/runner/browser/safari.dart b/lib/src/runner/browser/safari.dart
new file mode 100644
index 00000000..4ad28da4
--- /dev/null
+++ b/lib/src/runner/browser/safari.dart
@@ -0,0 +1,73 @@
+// Copyright (c) 2015, 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.
+
+library test.runner.browser.safari;
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+
+import '../../util/io.dart';
+import 'browser.dart';
+
+/// A class for running an instance of Safari.
+///
+/// Any errors starting or running the process are reported through [onExit].
+class Safari implements Browser {
+  /// The underlying process.
+  Process _process;
+
+  Future get onExit => _onExitCompleter.future;
+  final _onExitCompleter = new Completer();
+
+  /// A future that completes when the browser process has started.
+  ///
+  /// This is used to ensure that [close] works regardless of when it's called.
+  Future get _onProcessStarted => _onProcessStartedCompleter.future;
+  final _onProcessStartedCompleter = new Completer();
+
+  /// Starts a new instance of Safari open to the given [url], which may be a
+  /// [Uri] or a [String].
+  ///
+  /// If [executable] is passed, it's used as the Safari executable. Otherwise
+  /// the default executable name for the current OS will be used.
+  Safari(url, {String executable}) {
+    if (executable == null) {
+      executable = '/Applications/Safari.app/Contents/MacOS/Safari';
+    }
+
+    // Don't return a Future here because there's no need for the caller to wait
+    // for the process to actually start. They should just wait for the HTTP
+    // request instead.
+    withTempDir((dir) {
+      // Safari will only open files (not general URLs) via the command-line
+      // API, so we create a dummy file to redirect it to the page we actually
+      // want it to load.
+      var redirect = p.join(dir, 'redirect.html');
+      new File(redirect).writeAsStringSync(
+          "<script>location = " + JSON.encode(url.toString()) + "</script>");
+
+      return Process.start(executable, [redirect]).then((process) {
+        _process = process;
+        _onProcessStartedCompleter.complete();
+
+        // TODO(nweiz): the browser's standard output is almost always useless
+        // noise, but we should allow the user to opt in to seeing it.
+        return _process.exitCode;
+      });
+    }).then((exitCode) {
+      if (exitCode != 0) throw "Safari failed with exit code $exitCode.";
+    }).then(_onExitCompleter.complete)
+        .catchError(_onExitCompleter.completeError);
+  }
+
+  Future close() {
+    _onProcessStarted.then((_) => _process.kill());
+
+    // Swallow exceptions. The user should explicitly use [onExit] for these.
+    return onExit.catchError((_) {});
+  }
+}
diff --git a/lib/src/runner/browser/server.dart b/lib/src/runner/browser/server.dart
index ca1d64de..cdec66d7 100644
--- a/lib/src/runner/browser/server.dart
+++ b/lib/src/runner/browser/server.dart
@@ -31,6 +31,7 @@ import 'dartium.dart';
 import 'content_shell.dart';
 import 'firefox.dart';
 import 'phantom_js.dart';
+import 'safari.dart';
 
 /// A server that serves JS-compiled tests to browsers.
 ///
@@ -367,6 +368,7 @@ void main() {
       case TestPlatform.chrome: return new Chrome(url);
       case TestPlatform.phantomJS: return new PhantomJS(url);
       case TestPlatform.firefox: return new Firefox(url);
+      case TestPlatform.safari: return new Safari(url);
       default:
         throw new ArgumentError("$browser is not a browser.");
     }
diff --git a/test/runner/browser/runner_test.dart b/test/runner/browser/runner_test.dart
index 97d779ab..dadf5491 100644
--- a/test/runner/browser/runner_test.dart
+++ b/test/runner/browser/runner_test.dart
@@ -138,6 +138,12 @@ void main() {
       expect(result.exitCode, equals(0));
     });
 
+    test("on Safari", () {
+      new File(p.join(_sandbox, "test.dart")).writeAsStringSync(_success);
+      var result = _runUnittest(["-p", "safari", "test.dart"]);
+      expect(result.exitCode, equals(0));
+    }, testOn: "mac-os");
+
     test("on Dartium", () {
       new File(p.join(_sandbox, "test.dart")).writeAsStringSync(_success);
       var result = _runUnittest(["-p", "dartium", "test.dart"]);
@@ -193,6 +199,12 @@ void main() {
       expect(result.exitCode, equals(1));
     });
 
+    test("on Safari", () {
+      new File(p.join(_sandbox, "test.dart")).writeAsStringSync(_failure);
+      var result = _runUnittest(["-p", "safari", "test.dart"]);
+      expect(result.exitCode, equals(1));
+    }, testOn: "mac-os");
+
     test("on Dartium", () {
       new File(p.join(_sandbox, "test.dart")).writeAsStringSync(_failure);
       var result = _runUnittest(["-p", "dartium", "test.dart"]);
diff --git a/test/runner/browser/safari_test.dart b/test/runner/browser/safari_test.dart
new file mode 100644
index 00000000..f0422413
--- /dev/null
+++ b/test/runner/browser/safari_test.dart
@@ -0,0 +1,140 @@
+// Copyright (c) 2015, 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.
+
+@TestOn("vm && mac-os")
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:test/test.dart';
+import 'package:test/src/runner/browser/safari.dart';
+import 'package:test/src/util/io.dart';
+import 'package:shelf/shelf.dart' as shelf;
+import 'package:shelf/shelf_io.dart' as shelf_io;
+import 'package:shelf_web_socket/shelf_web_socket.dart';
+
+void main() {
+  group("running JavaScript", () {
+    // The JavaScript to serve in the server. We use actual JavaScript here to
+    // avoid the pain of compiling to JS in a test
+    var javaScript;
+
+    var servePage = (request) {
+      var path = request.url.path;
+
+      // We support both shelf 0.5.x and 0.6.x. The former has a leading "/"
+      // here, the latter does not.
+      if (path.startsWith("/")) path = path.substring(1);
+
+      if (path.isEmpty) {
+        return new shelf.Response.ok("""
+<!doctype html>
+<html>
+<head>
+  <script src="index.js"></script>
+</head>
+</html>
+""", headers: {'content-type': 'text/html'});
+      } else if (path == "index.js") {
+        return new shelf.Response.ok(javaScript,
+            headers: {'content-type': 'application/javascript'});
+      } else {
+        return new shelf.Response.notFound(null);
+      }
+    };
+
+    var server;
+    var webSockets;
+    setUp(() {
+      var webSocketsController = new StreamController();
+      webSockets = webSocketsController.stream;
+
+      return shelf_io.serve(
+          new shelf.Cascade()
+              .add(webSocketHandler(webSocketsController.add))
+              .add(servePage).handler,
+          'localhost', 0).then((server_) {
+        server = server_;
+      });
+    });
+
+    tearDown(() {
+      if (server != null) server.close();
+
+      javaScript = null;
+      server = null;
+      webSockets = null;
+    });
+
+    test("starts Safari with the given URL", () {
+      javaScript = '''
+var webSocket = new WebSocket(window.location.href.replace("http://", "ws://"));
+webSocket.addEventListener("open", function() {
+  webSocket.send("loaded!");
+});
+''';
+      var safari = new Safari(baseUrlForAddress(server.address, server.port));
+
+      return webSockets.first.then((webSocket) {
+        return webSocket.first.then(
+            (message) => expect(message, equals("loaded!")));
+      }).whenComplete(safari.close);
+    });
+
+    test("doesn't preserve state across runs", () {
+      javaScript = '''
+localStorage.setItem("data", "value");
+
+var webSocket = new WebSocket(window.location.href.replace("http://", "ws://"));
+webSocket.addEventListener("open", function() {
+  webSocket.send("done");
+});
+''';
+      var safari = new Safari(baseUrlForAddress(server.address, server.port));
+
+      var first = true;
+      webSockets.listen(expectAsync((webSocket) {
+        if (first) {
+          // The first request will set local storage data. We can't kill the
+          // old safari and start a new one until we're sure that that has
+          // finished.
+          webSocket.first.then((_) {
+            safari.close();
+
+            javaScript = '''
+var webSocket = new WebSocket(window.location.href.replace("http://", "ws://"));
+webSocket.addEventListener("open", function() {
+  webSocket.send(localStorage.getItem("data"));
+});
+''';
+            safari = new Safari(baseUrlForAddress(server.address, server.port));
+            first = false;
+          });
+        } else {
+          // The second request will return the local storage data. This should
+          // be null, indicating that no data was saved between runs.
+          expect(
+              webSocket.first
+                  .then((message) => expect(message, equals('null')))
+                  .whenComplete(safari.close),
+              completes);
+        }
+      }, count: 2));
+    });
+  });
+
+  test("a process can be killed synchronously after it's started", () {
+    return shelf_io.serve(expectAsync((_) {}, count: 0), 'localhost', 0)
+        .then((server) {
+      var safari = new Safari(baseUrlForAddress(server.address, server.port));
+      return safari.close().whenComplete(server.close);
+    });
+  });
+
+  test("reports an error in onExit", () {
+    var safari = new Safari("http://dart-lang.org",
+        executable: "_does_not_exist");
+    expect(safari.onExit, throwsA(new isInstanceOf<ProcessException>()));
+  });
+}
diff --git a/test/runner/runner_test.dart b/test/runner/runner_test.dart
index 95d828b8..68a00a2b 100644
--- a/test/runner/runner_test.dart
+++ b/test/runner/runner_test.dart
@@ -48,7 +48,7 @@ Usage: pub run test:test [files or directories...]
 
 -N, --plain-name               A plain-text substring of the name of the test to run.
 -p, --platform                 The platform(s) on which to run the tests.
-                               [vm (default), dartium, content-shell, chrome, phantomjs, firefox]
+                               [vm (default), dartium, content-shell, chrome, phantomjs, firefox, safari]
 
 -j, --concurrency=<threads>    The number of concurrent test suites run.
                                (defaults to $_defaultConcurrency)
-- 
GitLab