tools/testing/dart/browser_controller.dart - Issue 2901923003: Replace the configuration map with a typed object.

Unified Diff: tools/testing/dart/browser_controller.dart

Issue 2901923003: Replace the configuration map with a typed object. (Closed)

Patch Set: Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: tools/testing/dart/browser_controller.dart

diff --git a/tools/testing/dart/browser_controller.dart b/tools/testing/dart/browser_controller.dart

index 98c676484203190a4dd87d87afa83819f511fba4..4ff5d48e7126aa7a3eaa145a551b55fd7bf6c91c 100644

--- a/tools/testing/dart/browser_controller.dart

+++ b/tools/testing/dart/browser_controller.dart

@@ -1,21 +1,19 @@

// BSD-style license that can be found in the LICENSE file.

-library browser;

-import "dart:async";

-import "dart:convert" show UTF8, JSON;

-import "dart:core";

-import "dart:io";

-import "dart:math" show min;

+import 'dart:async';

+import 'dart:convert';

+import 'dart:core';

+import 'dart:io';

+import 'dart:math';

import 'android.dart';

-import 'http_server.dart';

+import 'configuration.dart';

import 'path.dart';

+import 'reset_safari.dart';

import 'utils.dart';

-import 'reset_safari.dart' show killAndResetSafari;

typedef void BrowserDoneCallback(BrowserTestOutput output);

typedef void TestChangedCallback(String browserId, String output, int testId);

typedef BrowserTest NextTestCallback(String browserId);

@@ -73,24 +71,34 @@ abstract class Browser {

Browser();

- factory Browser.byName(String name, String executablePath,

+ factory Browser.byRuntime(Runtime runtime, String executablePath,

[bool checkedMode = false]) {

Browser browser;

- if (name == 'firefox') {

- browser = new Firefox();

- } else if (name == 'chrome') {

- browser = new Chrome();

- } else if (name == 'dartium') {

- browser = new Dartium(checkedMode);

- } else if (name == 'safari') {

- browser = new Safari();

- } else if (name == 'safarimobilesim') {

- browser = new SafariMobileSimulator();

- } else if (name.startsWith('ie')) {

- browser = new IE();

- } else {

- throw "Non supported browser";

+ switch (runtime) {

+ case Runtime.firefox:

+ browser = new Firefox();

+ break;

+ case Runtime.chrome:

+ browser = new Chrome();

+ break;

+ case Runtime.dartium:

+ browser = new Dartium(checkedMode);

+ break;

+ case Runtime.safari:

+ browser = new Safari();

+ break;

+ case Runtime.safariMobileSim:

+ browser = new SafariMobileSimulator();

+ break;

+ case Runtime.ie9:

+ case Runtime.ie10:

+ case Runtime.ie11:

+ browser = new IE();

+ break;

+ default:

+ throw "unreachable";

}

browser._binary = executablePath;

return browser;

}

@@ -106,16 +114,6 @@ abstract class Browser {

'dartium'

];

- static const List<String> BROWSERS_WITH_WINDOW_SUPPORT = const [

- 'ie11',

- 'ie10'

- ];

- /// If [browserName] doesn't support Window.open, we use iframes instead.

- static bool requiresIframe(String browserName) {

- return !BROWSERS_WITH_WINDOW_SUPPORT.contains(browserName);

- }

static bool requiresFocus(String browserName) {

return browserName == "safari";

}

@@ -945,13 +943,11 @@ class BrowserTestRunner {

/// If the queue was recently empty, don't start another browser.

static const Duration MIN_NONEMPTY_QUEUE_TIME = const Duration(seconds: 1);

- final Map<String, dynamic> configuration;

+ final Configuration configuration;

final BrowserTestingServer testingServer;

final String localIp;

- final String browserName;

int maxNumBrowsers;

- final bool checkedMode;

int numBrowsers = 0;

// Used to send back logs from the browser (start, stop etc)

Function logger;

@@ -963,18 +959,18 @@ class BrowserTestRunner {

int numBrowserGetTestTimeouts = 0;

DateTime lastEmptyTestQueueTime = new DateTime.now();

String _currentStartingBrowserId;

- List<BrowserTest> testQueue = new List<BrowserTest>();

- Map<String, BrowserStatus> browserStatus = new Map<String, BrowserStatus>();

+ List<BrowserTest> testQueue = [];

+ Map<String, BrowserStatus> browserStatus = {};

- var adbDeviceMapping = new Map<String, AdbDevice>();

+ Map<String, AdbDevice> adbDeviceMapping = {};

List<AdbDevice> idleAdbDevices;

// This cache is used to guarantee that we never see double reporting.

// If we do we need to provide developers with this information.

// We don't add urls to the cache until we have run it.

- Map<int, String> testCache = new Map<int, String>();

+ Map<int, String> testCache = {};

- Map<int, String> doubleReportingOutputs = new Map<int, String>();

+ Map<int, String> doubleReportingOutputs = {};

List<String> timedOut = [];

// We will start a new browser when the test queue hasn't been empty

@@ -998,17 +994,12 @@ class BrowserTestRunner {

if (_currentStartingBrowserId == id) _currentStartingBrowserId = null;

}

- BrowserTestRunner(Map<String, dynamic> configuration, String localIp,

- String browserName, this.maxNumBrowsers)

+ BrowserTestRunner(

+ Configuration configuration, String localIp, this.maxNumBrowsers)

: configuration = configuration,

localIp = localIp,

- browserName = (browserName == 'ff') ? 'firefox' : browserName,

Bill Hesse 2017/05/29 13:08:27 I'm very happy about removing this line.

Bob Nystrom 2017/05/30 23:29:30 Alas, its legacy lives on to some degree in a few

- checkedMode = configuration['checked'] as bool,

- testingServer = new BrowserTestingServer(

- configuration,

- localIp,

- Browser.requiresIframe(browserName),

- Browser.requiresFocus(browserName)) {

+ testingServer = new BrowserTestingServer(configuration, localIp,

+ Browser.requiresFocus(configuration.runtime.name)) {

testingServer.testRunner = this;

}

@@ -1019,7 +1010,7 @@ class BrowserTestRunner {

..testStatusUpdateCallBack = handleStatusUpdate

..testStartedCallBack = handleStarted

..nextTestCallBack = getNextTest;

- if (browserName == 'chromeOnAndroid') {

+ if (configuration.runtime == Runtime.chromeOnAndroid) {

var idbNames = await AdbHelper.listDevices();

idleAdbDevices = new List.from(idbNames.map((id) => new AdbDevice(id)));

maxNumBrowsers = min(maxNumBrowsers, idleAdbDevices.length);

@@ -1047,21 +1038,24 @@ class BrowserTestRunner {

String getNextBrowserId() => "BROWSER${browserIdCounter++}";

void createBrowser() {

- final String id = getNextBrowserId();

- final String url = testingServer.getDriverUrl(id);

+ var id = getNextBrowserId();

+ var url = testingServer.getDriverUrl(id);

Browser browser;

- if (browserName == 'chromeOnAndroid') {

+ if (configuration.runtime == Runtime.chromeOnAndroid) {

AdbDevice device = idleAdbDevices.removeLast();

adbDeviceMapping[id] = device;

browser = new AndroidChrome(device);

} else {

- String path = Locations.getBrowserLocation(browserName, configuration);

- browser = new Browser.byName(browserName, path, checkedMode);

+ var path = configuration.browserLocation;

+ browser = new Browser.byRuntime(

+ configuration.runtime, path, configuration.isChecked);

browser.logger = logger;

}

browser.id = id;

markCurrentlyStarting(id);

- final status = new BrowserStatus(browser);

+ var status = new BrowserStatus(browser);

browserStatus[id] = status;

numBrowsers++;

status.nextTestTimeout = createNextTestTimer(status);

@@ -1188,7 +1182,7 @@ class BrowserTestRunner {

/// Remove a browser that has closed from our data structures that track

/// open browsers. Check if we want to replace it with a new browser.

void removeBrowser(String id) {

- if (browserName == 'chromeOnAndroid') {

+ if (configuration.runtime == Runtime.chromeOnAndroid) {

idleAdbDevices.add(adbDeviceMapping.remove(id));

}

markNotCurrentlyStarting(id);

@@ -1212,8 +1206,9 @@ class BrowserTestRunner {

// Restart Internet Explorer if it has been

// running for longer than RESTART_BROWSER_INTERVAL. The tests have

// had flaky timeouts, and this may help.

- if ((browserName == 'ie10' || browserName == 'ie11') &&

- status.timeSinceRestart.elapsed > RESTART_BROWSER_INTERVAL) {

+ if (configuration.runtime == Runtime.ie10 ||

Bill Hesse 2017/05/29 13:08:27 Need parentheses around the || expression: (a || b

Bob Nystrom 2017/05/30 23:29:30 Oh, dang. Nice catch. Done.

+ configuration.runtime == Runtime.ie11 &&

+ status.timeSinceRestart.elapsed > RESTART_BROWSER_INTERVAL) {

var id = status.browser.id;

// Reset stopwatch so we don't trigger again before restarting.

status.timeout = true;

@@ -1342,7 +1337,7 @@ class BrowserTestRunner {

}

class BrowserTestingServer {

- final Map configuration;

+ final Configuration configuration;

/// Interface of the testing server:

///

@@ -1357,7 +1352,6 @@ class BrowserTestingServer {

/// test

final String localIp;

- final bool useIframe;

final bool requiresFocus;

BrowserTestRunner testRunner;

@@ -1378,13 +1372,11 @@ class BrowserTestingServer {

TestChangedCallback testStartedCallBack;

NextTestCallback nextTestCallBack;

- BrowserTestingServer(

- this.configuration, this.localIp, this.useIframe, this.requiresFocus);

+ BrowserTestingServer(this.configuration, this.localIp, this.requiresFocus);

Future start() {

- var testDriverErrorPort = configuration['test_driver_error_port'] as int;

return HttpServer

- .bind(localIp, testDriverErrorPort)

+ .bind(localIp, configuration.testDriverErrorPort)

.then(setupErrorServer)

.then(setupDispatchingServer);

}

@@ -1418,7 +1410,7 @@ class BrowserTestingServer {

}

void setupDispatchingServer(_) {

- var server = (configuration['_servers_'] as TestingServers).server;

+ var server = configuration.servers.server;

void noCache(HttpRequest request) {

request.response.headers

.set("Cache-Control", "no-cache, no-store, must-revalidate");

@@ -1534,8 +1526,8 @@ class BrowserTestingServer {

exit(1);

// This should never happen - exit immediately;

}

- var port = (configuration['_servers_'] as TestingServers).port;

- return "http://$localIp:$port/driver/$browserId";

+ return "http://$localIp:${configuration.servers.port}/driver/$browserId";

}

Future<String> getDriverPage(String browserId) async {

@@ -1585,7 +1577,7 @@ body div {

var number_div = document.getElementById('number');

var executing_div = document.getElementById('currently_executing');

var error_div = document.getElementById('unhandled_error');

- var use_iframe = ${useIframe};

+ var use_iframe = ${configuration.runtime.requiresIFrame};

var start = new Date();

// Object that holds the state of an HTML test

« no previous file with comments | « no previous file | tools/testing/dart/co19_test.dart » ('j') | tools/testing/dart/configuration.dart » ('J')