Unverified Commit ec0b7443 authored by Victoria Ashworth's avatar Victoria Ashworth Committed by GitHub

Fix log filtering and CI tests for iOS 17 physical devices (#132491)

Fixes a couple of issues introduced in new iOS 17 physical device tooling: https://github.com/flutter/flutter/pull/131865.

1) Duplicate messages were being filtered out too aggressively. 

For example, if on the counter app, you printed "Increment!" on button click, it would only print once no matter how many times you clicked.

Sometimes more than one log source is used at a time and the original intention was to filter duplicates between two log sources, so it wouldn't print the same message from both logs. However, it would also filter when the same message was added more than once via the same log.

The new solution distinguishes a "primary" and a "fallback" log source and prefers to use the primary source unless it's not working, in which it'll use the fallback. If the fallback is faster than the primary, the primary will exclude the logs received by the fallback in a 1-to-1 fashion to prevent too-aggressive filtering. Once a flutter-message has been received by the primary source, fallback messages will be ignored.

Note: iOS < 17 did not regress.

2) There was a race condition between the shutdown hooks and exiting XcodeDebug that was causing a crash when deleting a file that doesn't exist. This only affects CI  - for the new integration tests and when testing with iOS 17 physical devices.
parent 8b8f262d
......@@ -7,6 +7,7 @@ import 'dart:async';
import 'package:meta/meta.dart';
import 'package:process/process.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart';
import '../base/io.dart';
import '../base/logger.dart';
......@@ -186,7 +187,12 @@ class XcodeDebug {
if (currentDebuggingProject != null) {
final XcodeDebugProject project = currentDebuggingProject!;
if (project.isTemporaryProject) {
project.xcodeProject.parent.deleteSync(recursive: true);
// Only delete if it exists. This is to prevent crashes when racing
// with shutdown hooks to delete temporary files.
ErrorHandlingFileSystem.deleteIfExists(
project.xcodeProject.parent,
recursive: true,
);
}
currentDebuggingProject = null;
}
......
......@@ -784,6 +784,69 @@ void main() {
expect(status, isTrue);
});
testWithoutContext('prints error message when deleting temporary directory that is nonexistant', () async {
final Xcode xcode = setupXcode(
fakeProcessManager: fakeProcessManager,
fileSystem: fileSystem,
flutterRoot: flutterRoot,
);
final XcodeDebugProject project = XcodeDebugProject(
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
isTemporaryProject: true,
);
final XcodeDebug xcodeDebug = XcodeDebug(
logger: logger,
processManager: fakeProcessManager,
xcode: xcode,
fileSystem: fileSystem,
);
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
'xcrun',
'osascript',
'-l',
'JavaScript',
pathToXcodeAutomationScript,
'stop',
'--xcode-path',
pathToXcodeApp,
'--project-path',
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--close-window'
],
stdout: '''
{"status":true,"errorMessage":null,"debugResult":null}
''',
),
]);
xcodeDebug.startDebugActionProcess = FakeProcess();
xcodeDebug.currentDebuggingProject = project;
expect(xcodeDebug.startDebugActionProcess, isNotNull);
expect(xcodeDebug.currentDebuggingProject, isNotNull);
expect(projectDirectory.existsSync(), isFalse);
expect(xcodeproj.existsSync(), isFalse);
expect(xcworkspace.existsSync(), isFalse);
final bool status = await xcodeDebug.exit(skipDelay: true);
expect((xcodeDebug.startDebugActionProcess! as FakeProcess).killed, isTrue);
expect(xcodeDebug.currentDebuggingProject, isNull);
expect(projectDirectory.existsSync(), isFalse);
expect(xcodeproj.existsSync(), isFalse);
expect(xcworkspace.existsSync(), isFalse);
expect(logger.errorText, contains('Failed to delete temporary Xcode project'));
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(status, isTrue);
});
testWithoutContext('kill Xcode when force exit', () async {
final Xcode xcode = setupXcode(
fakeProcessManager: FakeProcessManager.any(),
......@@ -825,6 +888,46 @@ void main() {
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(exitStatus, isTrue);
});
testWithoutContext('does not crash when deleting temporary directory that is nonexistant when force exiting', () async {
final Xcode xcode = setupXcode(
fakeProcessManager: FakeProcessManager.any(),
fileSystem: fileSystem,
flutterRoot: flutterRoot,
);
final XcodeDebugProject project = XcodeDebugProject(
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
isTemporaryProject: true,
);
final XcodeDebug xcodeDebug = XcodeDebug(
logger: logger,
processManager:FakeProcessManager.any(),
xcode: xcode,
fileSystem: fileSystem,
);
xcodeDebug.startDebugActionProcess = FakeProcess();
xcodeDebug.currentDebuggingProject = project;
expect(xcodeDebug.startDebugActionProcess, isNotNull);
expect(xcodeDebug.currentDebuggingProject, isNotNull);
expect(projectDirectory.existsSync(), isFalse);
expect(xcodeproj.existsSync(), isFalse);
expect(xcworkspace.existsSync(), isFalse);
final bool status = await xcodeDebug.exit(force: true);
expect((xcodeDebug.startDebugActionProcess! as FakeProcess).killed, isTrue);
expect(xcodeDebug.currentDebuggingProject, isNull);
expect(projectDirectory.existsSync(), isFalse);
expect(xcodeproj.existsSync(), isFalse);
expect(xcworkspace.existsSync(), isFalse);
expect(logger.errorText, isEmpty);
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(status, isTrue);
});
});
group('stop app', () {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment