Skip to content

Add support for expression compilation when debugging integration tests #113481

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 11 additions & 9 deletions packages/flutter_tools/lib/src/test/flutter_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ class FlutterPlatform extends PlatformPlugin {
debuggingOptions: debuggingOptions,
device: integrationTestDevice!,
userIdentifier: integrationTestUserIdentifier,
compileExpression: _compileExpressionService
);
}
return FlutterTesterTestDevice(
Expand Down Expand Up @@ -457,21 +458,22 @@ class FlutterPlatform extends PlatformPlugin {
controllerSinkClosed = true;
}));

// When start paused is specified, it means that the user is likely
// running this with a debugger attached. Initialize the resident
// compiler in this case.
if (debuggingOptions.startPaused) {
compiler ??= TestCompiler(debuggingOptions.buildInfo, flutterProject, precompiledDillPath: precompiledDillPath, testTimeRecorder: testTimeRecorder);
final Uri testUri = globals.fs.file(testPath).uri;
// Trigger a compilation to initialize the resident compiler.
unawaited(compiler!.compile(testUri));
}

// If a kernel file is given, then use that to launch the test.
// If mapping is provided, look kernel file from mapping.
// If all fails, create a "listener" dart that invokes actual test.
String? mainDart;
if (precompiledDillPath != null) {
mainDart = precompiledDillPath;
// When start paused is specified, it means that the user is likely
// running this with a debugger attached. Initialize the resident
// compiler in this case.
if (debuggingOptions.startPaused) {
compiler ??= TestCompiler(debuggingOptions.buildInfo, flutterProject, precompiledDillPath: precompiledDillPath, testTimeRecorder: testTimeRecorder);
final Uri testUri = globals.fs.file(testPath).uri;
// Trigger a compilation to initialize the resident compiler.
unawaited(compiler!.compile(testUri));
}
} else if (precompiledDillFiles != null) {
mainDart = precompiledDillFiles![testPath];
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ class IntegrationTestTestDevice implements TestDevice {
required this.device,
required this.debuggingOptions,
required this.userIdentifier,
required this.compileExpression,
});

final int id;
final Device device;
final DebuggingOptions debuggingOptions;
final String? userIdentifier;
final CompileExpression? compileExpression;

ApplicationPackage? _applicationPackage;
final Completer<void> _finished = Completer<void>();
Expand Down Expand Up @@ -70,7 +72,11 @@ class IntegrationTestTestDevice implements TestDevice {
_gotProcessObservatoryUri.complete(observatoryUri);

globals.printTrace('test $id: Connecting to vm service');
final FlutterVmService vmService = await connectToVmService(observatoryUri, logger: globals.logger).timeout(
final FlutterVmService vmService = await connectToVmService(
observatoryUri,
logger: globals.logger,
compileExpression: compileExpression,
).timeout(
const Duration(seconds: 5),
onTimeout: () => throw TimeoutException('Connecting to the VM Service timed out.'),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ void main() {
BuildInfo.debug,
),
userIdentifier: '',
compileExpression: null,
);

fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
Expand Down Expand Up @@ -173,6 +174,7 @@ void main() {
BuildInfo.debug,
),
userIdentifier: '',
compileExpression: null,
);

expect(() => testDevice.start('entrypointPath'), throwsA(isA<TestDeviceException>()));
Expand Down Expand Up @@ -201,6 +203,7 @@ void main() {
BuildInfo.debug,
),
userIdentifier: '',
compileExpression: null,
);

expect(() => testDevice.start('entrypointPath'), throwsA(isA<TestDeviceException>()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:vm_service/vm_service.dart';

import '../src/common.dart';
import 'test_data/basic_project.dart';
import 'test_data/integration_tests_project.dart';
import 'test_data/tests_project.dart';
import 'test_driver.dart';
import 'test_utils.dart';
Expand Down Expand Up @@ -141,6 +142,37 @@ void batch2() {
});
}

void batch3() {
final IntegrationTestsProject project = IntegrationTestsProject();
late Directory tempDir;
late FlutterTestTestDriver flutter;

Future<void> initProject() async {
tempDir = createResolvedTempDirectorySync('integration_test_expression_eval_test.');
await project.setUpIn(tempDir);
flutter = FlutterTestTestDriver(tempDir);
}

Future<void> cleanProject() async {
await flutter.waitForCompletion();
tryToDelete(tempDir);
}

testWithoutContext('flutter integration test expression evaluation - can evaluate expressions in a test', () async {
await initProject();
await flutter.test(
deviceId: 'flutter-tester',
testFile: project.testFilePath,
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
);
await flutter.waitForPause();
await evaluateTrivialExpressions(flutter);
await cleanProject();
});

}

Future<void> evaluateTrivialExpressions(FlutterTestDriver flutter) async {
ObjRef res;

Expand Down Expand Up @@ -189,4 +221,5 @@ void expectValue(ObjRef result, String message) {
void main() {
batch1();
batch2();
batch3();
Copy link
Contributor

Choose a reason for hiding this comment

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

you have any idea why we batch them like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not for certain.. They were added in https://github.com/flutter/flutter/pull/41880/files

I figured maybe it was for some consistency and scoping. Those in each batch use a different TestProject and each "batch" has a similar initProject, cleanProject function. I'm not sure I see a reason why they wouldn't have just been inside group()s though. I can try changing them to be that way though (either in this PR or another)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gonna merge this, but let me know if you think I should try changing these to groups in a future CL. Base on other things in the change above, maybe this was intended to provide some sharding, but as far as I can tell nobody is calling these batch() functions individually.

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ class IntegrationTestsProject extends Project implements TestsProject {
String get testFilePath => fileSystem.path.join(dir.path, 'integration_test', 'app_test.dart');

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

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

@override
int get breakpointLine => throw UnimplementedError();
int get breakpointLine => lineContaining(testContent, '// BREAKPOINT');
}
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ class FlutterTestTestDriver extends FlutterTestDriver {

Future<void> test({
String testFile = 'test/test.dart',
String? deviceId,
bool withDebugger = false,
bool pauseOnExceptions = false,
bool coverage = false,
Expand All @@ -775,6 +776,8 @@ class FlutterTestTestDriver extends FlutterTestDriver {
'--machine',
if (coverage)
'--coverage',
if (deviceId != null)
...<String>['-d', deviceId],
], script: testFile, withDebugger: withDebugger, pauseOnExceptions: pauseOnExceptions, beforeStart: beforeStart);
}

Expand Down