Skip to content

Migrate test/fixtures/context.dart to null-safety #1698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 30, 2022
Merged

Migrate test/fixtures/context.dart to null-safety #1698

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f8bf37
Migrate test/fixtures/context.dart to null-safety
elliette Jul 21, 2022
841b6ab
Respond to PR comments
elliette Jul 25, 2022
b299858
Test server is not late
elliette Jul 25, 2022
88f6328
Resolve conflicts
elliette Jul 25, 2022
54ed65c
Fix test
elliette Jul 27, 2022
fd828d2
Fix frontend tests
elliette Jul 27, 2022
1600c1d
Merge branch 'master' into null-safe-10
elliette Jul 28, 2022
d45165d
urlEncoder can be null
elliette Jul 28, 2022
3edd446
Switch daemonClient to a getter"
elliette Jul 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 78 additions & 71 deletions dwds/test/fixtures/context.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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.

// @dart = 2.9

import 'dart:async';
import 'dart:convert';
import 'dart:io';
Expand Down Expand Up @@ -56,45 +54,53 @@ enum IndexBaseMode { noBase, base }
enum NullSafety { weak, sound }

class TestContext {
String appUrl;
WipConnection tabConnection;
WipConnection extensionConnection;
TestServer testServer;
BuildDaemonClient daemonClient;
ResidentWebRunner webRunner;
WebDriver webDriver;
Process chromeDriver;
AppConnection appConnection;
DebugConnection debugConnection;
WebkitDebugger webkitDebugger;
Client client;
ExpressionCompilerService ddcService;
int port;
Directory _outputDir;
File _entryFile;
Uri _packageConfigFile;
Uri _projectDirectory;
String _entryContents;

/// Null safety mode for the frontend server.
///
/// Note: flutter's frontend server is always launched with
/// the null safety setting inferred from project configurations
/// or the source code. We skip this inference and just set it
/// here to the desired value manually.
///
/// Note: build_runner-based setups ignore this setting and read
/// this value from the ddc debug metadata and pass it to the
/// expression compiler worker initialization API.
///
/// TODO(annagrin): Currently setting sound null safety for frontend
/// server tests fails due to missing sound SDK JavaScript and maps.
/// Issue: https://github.com/dart-lang/webdev/issues/1591
NullSafety nullSafety;
String get appUrl => _appUrl!;
late String? _appUrl;

WipConnection get tabConnection => _tabConnection!;
late WipConnection? _tabConnection;

TestServer get testServer => _testServer!;
TestServer? _testServer;

BuildDaemonClient get daemonClient => _daemonClient!;
BuildDaemonClient? _daemonClient;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to make _daemonClient logic similar to other fields here?


ResidentWebRunner get webRunner => _webRunner!;
ResidentWebRunner? _webRunner;

WebDriver get webDriver => _webDriver!;
WebDriver? _webDriver;

Process get chromeDriver => _chromeDriver!;
late Process? _chromeDriver;

WebkitDebugger get webkitDebugger => _webkitDebugger!;
late WebkitDebugger? _webkitDebugger;

Client get client => _client!;
late Client? _client;

ExpressionCompilerService? ddcService;

int get port => _port!;
late int? _port;

Directory get outputDir => _outputDir!;
late Directory? _outputDir;

late WipConnection extensionConnection;
late AppConnection appConnection;
late DebugConnection debugConnection;
late File _entryFile;
late Uri _packageConfigFile;
late Uri _projectDirectory;
late String _entryContents;

final _logger = logging.Logger('Context');

/// Top level directory in which we run the test server..
String workingDirectory;
late String workingDirectory;

/// The path to build and serve.
String pathToServe;
Expand All @@ -103,8 +109,8 @@ class TestContext {
String path;

TestContext(
{String directory,
String entry,
{String? directory,
String? entry,
this.path = 'hello_world/index.html',
this.pathToServe = 'example'}) {
final relativeDirectory = p.join('..', 'fixtures', '_test');
Expand Down Expand Up @@ -144,19 +150,19 @@ class TestContext {
bool spawnDds = true,
String hostname = 'localhost',
bool waitToDebug = false,
UrlEncoder urlEncoder,
UrlEncoder? urlEncoder,
CompilationMode compilationMode = CompilationMode.buildDaemon,
NullSafety nullSafety = NullSafety.weak,
bool enableExpressionEvaluation = false,
bool verboseCompiler = false,
SdkConfigurationProvider sdkConfigurationProvider,
SdkConfigurationProvider? sdkConfigurationProvider,
}) async {
sdkConfigurationProvider ??= DefaultSdkConfigurationProvider();

try {
configureLogWriter();

client = IOClient(HttpClient()
_client = IOClient(HttpClient()
..maxConnectionsPerHost = 200
..idleTimeout = const Duration(seconds: 30)
..connectionTimeout = const Duration(seconds: 30));
Expand All @@ -167,7 +173,7 @@ class TestContext {
final chromeDriverPort = await findUnusedPort();
final chromeDriverUrlBase = 'wd/hub';
try {
chromeDriver = await Process.start('chromedriver$_exeExt',
_chromeDriver = await Process.start('chromedriver$_exeExt',
['--port=$chromeDriverPort', '--url-base=$chromeDriverUrlBase']);
// On windows this takes a while to boot up, wait for the first line
// of stdout as a signal that it is ready.
Expand Down Expand Up @@ -195,14 +201,14 @@ class TestContext {
await Process.run(dartPath, ['pub', 'upgrade'],
workingDirectory: workingDirectory);

ExpressionCompiler expressionCompiler;
ExpressionCompiler? expressionCompiler;
AssetReader assetReader;
Handler assetHandler;
Stream<BuildResults> buildResults;
RequireStrategy requireStrategy;
String basePath = '';

port = await findUnusedPort();
_port = await findUnusedPort();
switch (compilationMode) {
case CompilationMode.buildDaemon:
{
Expand All @@ -213,7 +219,7 @@ class TestContext {
],
'--verbose',
];
daemonClient =
_daemonClient =
await connectClient(workingDirectory, options, (log) {
final record = log.toLogRecord();
final name =
Expand Down Expand Up @@ -263,14 +269,14 @@ class TestContext {
final entry = p.toUri(_entryFile.path
.substring(_projectDirectory.toFilePath().length + 1));

webRunner = ResidentWebRunner(
_webRunner = ResidentWebRunner(
entry,
urlEncoder,
_projectDirectory,
_packageConfigFile,
[_projectDirectory],
'org-dartlang-app',
_outputDir.path,
outputDir.path,
nullSafety == NullSafety.sound,
verboseCompiler,
);
Expand Down Expand Up @@ -315,14 +321,14 @@ class TestContext {
]
}
});
webDriver = await createDriver(
_webDriver = await createDriver(
spec: WebDriverSpec.JsonWire,
desired: capabilities,
uri: Uri.parse(
'http://127.0.0.1:$chromeDriverPort/$chromeDriverUrlBase/'));
final connection = ChromeConnection('localhost', debugPort);

testServer = await TestServer.start(
_testServer = await TestServer.start(
hostname,
port,
assetHandler,
Expand All @@ -342,13 +348,14 @@ class TestContext {
ddcService,
);

appUrl = basePath.isEmpty
_appUrl = basePath.isEmpty
? 'http://localhost:$port/$path'
: 'http://localhost:$port/$basePath/$path';

await webDriver.get(appUrl);
final tab = await connection.getTab((t) => t.url == appUrl);
tabConnection = await tab.connect();
await _webDriver?.get(appUrl);
final tab = await (connection.getTab((t) => t.url == appUrl)
as FutureOr<ChromeTab>);
_tabConnection = await tab.connect();
await tabConnection.runtime.enable();
await tabConnection.debugger.enable();

Expand All @@ -370,30 +377,30 @@ class TestContext {

Future<void> startDebugging() async {
debugConnection = await testServer.dwds.debugConnection(appConnection);
webkitDebugger = WebkitDebugger(WipDebugger(tabConnection));
_webkitDebugger = WebkitDebugger(WipDebugger(tabConnection));
}

Future<void> tearDown() async {
await webDriver?.quit(closeSession: true);
chromeDriver?.kill();
await _webDriver?.quit(closeSession: true);
_chromeDriver?.kill();
DartUri.currentDirectory = p.current;
_entryFile.writeAsStringSync(_entryContents);
await daemonClient?.close();
await _daemonClient?.close();
await ddcService?.stop();
await webRunner?.stop();
await testServer?.stop();
client?.close();
await _webRunner?.stop();
await _testServer?.stop();
_client?.close();
await _outputDir?.delete(recursive: true);
stopLogWriter();

// clear the state for next setup
webDriver = null;
chromeDriver = null;
daemonClient = null;
_webDriver = null;
_chromeDriver = null;
_daemonClient = null;
ddcService = null;
webRunner = null;
testServer = null;
client = null;
_webRunner = null;
_testServer = null;
_client = null;
_outputDir = null;
}

Expand All @@ -402,7 +409,7 @@ class TestContext {
_entryContents.replaceAll('Hello World!', 'Gary is awesome!'));

// Wait for the build.
await daemonClient.buildResults.firstWhere((results) => results.results
await _daemonClient?.buildResults.firstWhere((results) => results.results
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can _daemonClient be null here? If it could, I'd presume the tests might not work as there are no build results, so we probably should fail in that case anyway.
Btw, I see that _daemonClient is treated differently than the other fields of TestContext (ie there is no getter that asserts that _daemonClient is non-null) - is there a reason for the difference?

.any((result) => result.status == BuildStatus.succeeded));

// Allow change to propagate to the browser.
Expand Down Expand Up @@ -438,8 +445,8 @@ class TestContext {
Future<int> findBreakpointLine(
String breakpointId, String isolateId, ScriptRef scriptRef) async {
final script = await debugConnection.vmService
.getObject(isolateId, scriptRef.id) as Script;
final lines = LineSplitter.split(script.source).toList();
.getObject(isolateId, scriptRef.id!) as Script;
final lines = LineSplitter.split(script.source!).toList();
final lineNumber =
lines.indexWhere((l) => l.endsWith('// Breakpoint: $breakpointId'));
if (lineNumber == -1) {
Expand Down