Skip to content

Commit

Permalink
Fix issues running integration tests through DAP debug adapter (#104618)
Browse files Browse the repository at this point in the history
* Fix issues running integration tests through DAP

These adapters were incorrectly trying to connect a DDS instance even when Flutter would create its own. This change disables DDS in the DAP layer and leaves it to Flutter (although it passes `--no-dds` on to Flutter if provided to the DAP process).

Also fixes an issue where we would unnecessarily connect the VM Service for tests even in 'noDebug' mode because of a change/fix that now includes a 'vmServiceUri' in the `test.startedProcess` event.
  • Loading branch information
DanTup authored May 26, 2022
1 parent 2aa348b commit 1470203
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 46 deletions.
15 changes: 13 additions & 2 deletions packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,24 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments
required this.fileSystem,
required this.platform,
super.ipv6,
super.enableDds,
bool enableDds = true,
super.enableAuthCodes,
super.logger,
});
}) : _enableDds = enableDds,
// Always disable in the DAP layer as it's handled in the spawned
// 'flutter' process.
super(enableDds: false);

FileSystem fileSystem;
Platform platform;
Process? _process;

/// Whether DDS should be enabled in the Flutter process.
///
/// We never enable DDS in the DAP process for Flutter, so this value is not
/// the same as what is passed to the base class, which is always provided 'false'.
final bool _enableDds;

@override
final FlutterLaunchRequestArguments Function(Map<String, Object?> obj)
parseLaunchArgs = FlutterLaunchRequestArguments.fromJson;
Expand Down Expand Up @@ -136,6 +145,7 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments
final List<String> toolArgs = <String>[
'attach',
'--machine',
if (!_enableDds) '--no-dds',
if (vmServiceUri != null)
...<String>['--debug-uri', vmServiceUri],
];
Expand Down Expand Up @@ -249,6 +259,7 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments
final List<String> toolArgs = <String>[
'run',
'--machine',
if (!_enableDds) '--no-dds',
if (enableDebugger) '--start-paused',
// Structured errors are enabled by default, but since we don't connect
// the VM Service for noDebug, we need to disable them so that error text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,24 @@ class FlutterTestDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArgum
required this.fileSystem,
required this.platform,
super.ipv6,
super.enableDds,
bool enableDds = true,
super.enableAuthCodes,
super.logger,
});
}) : _enableDds = enableDds,
// Always disable in the DAP layer as it's handled in the spawned
// 'flutter' process.
super(enableDds: false);

FileSystem fileSystem;
Platform platform;
Process? _process;

/// Whether DDS should be enabled in the Flutter process.
///
/// We never enable DDS in the DAP process for Flutter, so this value is not
/// the same as what is passed to the base class, which is always provided 'false'.
final bool _enableDds;

@override
final FlutterLaunchRequestArguments Function(Map<String, Object?> obj)
parseLaunchArgs = FlutterLaunchRequestArguments.fromJson;
Expand Down Expand Up @@ -78,6 +87,22 @@ class FlutterTestDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArgum
terminatePids(ProcessSignal.sigkill);
}

/// Whether or not the user requested debugging be enabled.
///
/// For debugging to be enabled, the user must have chosen "Debug" (and not
/// "Run") in the editor (which maps to the DAP `noDebug` field).
bool get enableDebugger {
final DartCommonLaunchAttachRequestArguments args = this.args;
if (args is FlutterLaunchRequestArguments) {
// Invert DAP's noDebug flag, treating it as false (so _do_ debug) if not
// provided.
return !(args.noDebug ?? false);
}

// Otherwise (attach), always debug.
return true;
}

/// Called by [launchRequest] to request that we actually start the tests to be run/debugged.
///
/// For debugging, this should start paused, connect to the VM Service, set
Expand All @@ -86,12 +111,13 @@ class FlutterTestDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArgum
Future<void> launchImpl() async {
final FlutterLaunchRequestArguments args = this.args as FlutterLaunchRequestArguments;

final bool debug = !(args.noDebug ?? false);
final bool debug = enableDebugger;
final String? program = args.program;

final List<String> toolArgs = <String>[
'test',
'--machine',
if (!_enableDds) '--no-dds',
if (debug) '--start-paused',
];

Expand Down Expand Up @@ -207,8 +233,9 @@ class FlutterTestDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArgum
/// Handles the test.processStarted event from Flutter that provides the VM Service URL.
void _handleTestStartedProcess(Map<String, Object?> params) {
final String? vmServiceUriString = params['observatoryUri'] as String?;
// For no-debug mode, this event is still sent, but has a null URI.
if (vmServiceUriString == null) {
// For no-debug mode, this event may be still sent so ignore it if we know
// we're not debugging, or its URI is null.
if (!enableDebugger || vmServiceUriString == null) {
return;
}
final Uri vmServiceUri = Uri.parse(vmServiceUriString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'package:file/file.dart';
import 'package:flutter_tools/src/cache.dart';

import '../../src/common.dart';
import '../test_data/integration_tests_project.dart';
import '../test_data/tests_project.dart';
import '../test_utils.dart';
import 'test_client.dart';
Expand All @@ -15,6 +16,8 @@ import 'test_support.dart';
void main() {
late Directory tempDir;
late DapTestSession dap;
late DapTestClient client;
late TestsProject project;

setUpAll(() {
Cache.flutterRoot = getFlutterRoot();
Expand All @@ -23,52 +26,48 @@ void main() {
setUp(() async {
tempDir = createResolvedTempDirectorySync('flutter_test_adapter_test.');
dap = await DapTestSession.setUp(additionalArgs: <String>['--test']);
client = dap.client;
});

tearDown(() async {
await dap.tearDown();
tryToDelete(tempDir);
});

test('can run in debug mode', () async {
final DapTestClient client = dap.client;
final TestsProject project = TestsProject();
await project.setUpIn(tempDir);

// Collect output and test events while running the script.
final TestEvents outputEvents = await client.collectTestOutput(
launch: () => client.launch(
program: project.testFilePath,
cwd: project.dir.path,
),
);

// Check the printed output shows that the run finished, and it's exit
// code (which is 1 due to the failing test).
final String output = outputEvents.output.map((OutputEventBody e) => e.output).join();
expectLines(
output,
<Object>[
startsWith('Connecting to VM Service at'),
..._testsProjectExpectedOutput,
],
allowExtras: true, // Allow for printed call stack etc.
);
void standardTests({List<String>? toolArgs}) {
test('can run in debug mode', () async {
// Collect output and test events while running the script.
final TestEvents outputEvents = await client.collectTestOutput(
launch: () => client.launch(
program: project.testFilePath,
cwd: project.dir.path,
toolArgs: toolArgs,
),
);

// Check the printed output shows that the run finished, and it's exit
// code (which is 1 due to the failing test).
final String output = outputEvents.output.map((OutputEventBody e) => e.output).join();
expectLines(
output,
<Object>[
startsWith('Connecting to VM Service at'),
..._testsProjectExpectedOutput,
],
allowExtras: true, // Allow for printed call stack etc.
);

_expectStandardTestsProjectResults(outputEvents);
});
_expectStandardTestsProjectResults(outputEvents);
});

test('can run in noDebug mode', () async {
final DapTestClient client = dap.client;
final TestsProject project = TestsProject();
await project.setUpIn(tempDir);

// Collect output and test events while running the script.
final TestEvents outputEvents = await client.collectTestOutput(
launch: () => client.launch(
program: project.testFilePath,
noDebug: true,
cwd: project.dir.path,
toolArgs: toolArgs,
),
);

Expand All @@ -85,21 +84,18 @@ void main() {
});

test('can run a single test', () async {
final DapTestClient client = dap.client;
final TestsProject project = TestsProject();
await project.setUpIn(tempDir);

// Collect output and test events while running the script.
final TestEvents outputEvents = await client.collectTestOutput(
launch: () => client.launch(
program: project.testFilePath,
noDebug: true,
cwd: project.dir.path,
// It's up to the calling IDE to pass the correct args for 'dart test'
// if it wants to run a subset of tests.
args: <String>[
// It's up to the calling IDE to pass the correct args for
// 'flutter test' if it wants to run a subset of tests.
toolArgs: <String>[
'--plain-name',
'can pass',
...?toolArgs,
],
),
);
Expand All @@ -112,6 +108,27 @@ void main() {
expect(testsNames, contains('Flutter tests can pass'));
expect(testsNames, isNot(contains('Flutter tests can fail')));
});
}

group('widget tests', () {
setUp(() async {
project = TestsProject();
await project.setUpIn(tempDir);
});

standardTests();
});

group('integration tests', () {
const List<String> toolArgs = <String>['-d', 'flutter-tester'];

setUp(() async {
project = IntegrationTestsProject();
await project.setUpIn(tempDir);
});

standardTests(toolArgs: toolArgs);
});
}

/// Matchers for the expected console output of [TestsProject].
Expand All @@ -137,8 +154,9 @@ final List<Object> _testsProjectExpectedOutput = <Object>[
void _expectStandardTestsProjectResults(TestEvents events) {
// Check we received all expected test events passed through from
// package:test.
final List<Object> eventNames =
events.testNotifications.map((Map<String, Object?> e) => e['type']!).toList();
final List<Object> eventNames = events.testNotifications
.map((Map<String, Object?> e) => e['type']!)
.toList();

// start/done should always be first/last.
expect(eventNames.first, equals('start'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class InProcessDapTestServer extends DapTestServer {
// Simulate flags based on the args to aid testing.
enableDds: !args.contains('--no-dds'),
ipv6: args.contains('--ipv6'),
test: args.contains('--test'),
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:file/file.dart';
import 'package:flutter_tools/src/base/file_system.dart';

import '../test_utils.dart';
import 'project.dart';
import 'tests_project.dart';

class IntegrationTestsProject extends Project implements TestsProject {
@override
final String pubspec = '''
name: test
environment:
sdk: '>=2.12.0-0 <3.0.0'
dependencies:
flutter:
sdk: flutter
dev_dependencies:
integration_test:
sdk: flutter
flutter_test:
sdk: flutter
''';

@override
String get main => '// Unused';

@override
final String testContent = r'''
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
void main() {
group('Flutter tests', () {
testWidgets('can pass', (WidgetTester tester) async {
expect(true, isTrue); // BREAKPOINT
});
testWidgets('can fail', (WidgetTester tester) async {
expect(true, isFalse);
});
});
}
''';

@override
Future<void> setUpIn(Directory dir) {
this.dir = dir;
writeFile(testFilePath, testContent);
return super.setUpIn(dir);
}

@override
String get testFilePath => fileSystem.path.join(dir.path, 'integration_test', 'app_test.dart');

@override
Uri get breakpointUri => throw UnimplementedError();

@override
Uri get breakpointAppUri => throw UnimplementedError();

@override
int get breakpointLine => throw UnimplementedError();
}

0 comments on commit 1470203

Please sign in to comment.