Unverified Commit 533cd7a6 authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tools] Delete system temp entries on fatal signals (#55513)

parent ea0c73c1
...@@ -3,13 +3,19 @@ ...@@ -3,13 +3,19 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:file/file.dart'; import 'package:file/file.dart';
import 'package:file/local.dart' as local_fs;
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'common.dart' show throwToolExit; import 'common.dart' show throwToolExit;
import 'io.dart';
import 'platform.dart'; import 'platform.dart';
import 'process.dart';
import 'signals.dart';
// package:file/local.dart must not be exported. This exposes LocalFileSystem,
// which we override to ensure that temporary directories are cleaned up when
// the tool is killed by a signal.
export 'package:file/file.dart'; export 'package:file/file.dart';
export 'package:file/local.dart';
/// Exception indicating that a file that was expected to exist was not found. /// Exception indicating that a file that was expected to exist was not found.
class FileNotFoundException implements IOException { class FileNotFoundException implements IOException {
...@@ -136,11 +142,94 @@ class FileSystemUtils { ...@@ -136,11 +142,94 @@ class FileSystemUtils {
/// Return the absolute path of the user's home directory /// Return the absolute path of the user's home directory
String get homeDirPath { String get homeDirPath {
String path = _platform.isWindows String path = _platform.isWindows
? _platform.environment['USERPROFILE'] ? _platform.environment['USERPROFILE']
: _platform.environment['HOME']; : _platform.environment['HOME'];
if (path != null) { if (path != null) {
path = _fileSystem.path.absolute(path); path = _fileSystem.path.absolute(path);
} }
return path; return path;
} }
} }
/// This class extends [local_fs.LocalFileSystem] in order to clean up
/// directories and files that the tool creates under the system temporary
/// directory when the tool exits either normally or when killed by a signal.
class LocalFileSystem extends local_fs.LocalFileSystem {
LocalFileSystem._(Signals signals, List<ProcessSignal> fatalSignals) :
_signals = signals, _fatalSignals = fatalSignals;
@visibleForTesting
LocalFileSystem.test({
@required Signals signals,
List<ProcessSignal> fatalSignals = Signals.defaultExitSignals,
}) : this._(signals, fatalSignals);
// Unless we're in a test of this class's signal hanlding features, we must
// have only one instance created with the singleton LocalSignals instance
// and the catchable signals it considers to be fatal.
static LocalFileSystem _instance;
static LocalFileSystem get instance => _instance ??= LocalFileSystem._(
LocalSignals.instance,
Signals.defaultExitSignals,
);
Directory _systemTemp;
final Map<ProcessSignal, Object> _signalTokens = <ProcessSignal, Object>{};
@visibleForTesting
static Future<void> dispose() => LocalFileSystem.instance?._dispose();
Future<void> _dispose() async {
_tryToDeleteTemp();
for (final MapEntry<ProcessSignal, Object> signalToken in _signalTokens.entries) {
await _signals.removeHandler(signalToken.key, signalToken.value);
}
_signalTokens.clear();
}
final Signals _signals;
final List<ProcessSignal> _fatalSignals;
void _tryToDeleteTemp() {
try {
if (_systemTemp?.existsSync() ?? false) {
_systemTemp.deleteSync(recursive: true);
}
} on FileSystemException {
// ignore.
}
_systemTemp = null;
}
// This getter returns a fresh entry under /tmp, like
// /tmp/flutter_tools.abcxyz, then the rest of the tool creates /tmp entries
// under that, like /tmp/flutter_tools.abcxyz/flutter_build_stuff.123456.
// Right before exiting because of a signal or otherwise, we delete
// /tmp/flutter_tools.abcxyz, not the whole of /tmp.
@override
Directory get systemTempDirectory {
if (_systemTemp == null) {
_systemTemp = super.systemTempDirectory.createTempSync(
'flutter_tools.',
)..createSync(recursive: true);
// Make sure that the temporary directory is cleaned up if the tool is
// killed by a signal.
for (final ProcessSignal signal in _fatalSignals) {
final Object token = _signals.addHandler(
signal,
(ProcessSignal _) {
_tryToDeleteTemp();
},
);
_signalTokens[signal] = token;
}
// Make sure that the temporary directory is cleaned up when the tool
// exits normally.
shutdownHooks?.addShutdownHook(
_tryToDeleteTemp,
ShutdownStage.CLEANUP,
);
}
return _systemTemp;
}
}
...@@ -90,6 +90,7 @@ export 'dart:io' ...@@ -90,6 +90,7 @@ export 'dart:io'
ProcessStartMode, ProcessStartMode,
// RandomAccessFile NO! Use `file_system.dart` // RandomAccessFile NO! Use `file_system.dart`
ServerSocket, ServerSocket,
SignalException,
// stderr, NO! Use `io.dart` // stderr, NO! Use `io.dart`
// stdin, NO! Use `io.dart` // stdin, NO! Use `io.dart`
Stdin, Stdin,
......
...@@ -4,30 +4,27 @@ ...@@ -4,30 +4,27 @@
import 'dart:async'; import 'dart:async';
import 'package:meta/meta.dart';
import 'async_guard.dart'; import 'async_guard.dart';
import 'context.dart';
import 'io.dart'; import 'io.dart';
typedef SignalHandler = FutureOr<void> Function(ProcessSignal signal); typedef SignalHandler = FutureOr<void> Function(ProcessSignal signal);
Signals get signals => Signals.instance;
// The default list of signals that should cause the process to exit.
const List<ProcessSignal> _defaultExitSignals = <ProcessSignal>[
ProcessSignal.SIGTERM,
ProcessSignal.SIGINT,
ProcessSignal.SIGKILL,
];
/// A class that manages signal handlers /// A class that manages signal handlers
/// ///
/// Signal handlers are run in the order that they were added. /// Signal handlers are run in the order that they were added.
abstract class Signals { abstract class Signals {
factory Signals({ @visibleForTesting
List<ProcessSignal> exitSignals = _defaultExitSignals, factory Signals.test({
}) => _DefaultSignals._(exitSignals); List<ProcessSignal> exitSignals = defaultExitSignals,
}) => LocalSignals._(exitSignals);
static Signals get instance => context.get<Signals>(); // The default list of signals that should cause the process to exit.
static const List<ProcessSignal> defaultExitSignals = <ProcessSignal>[
ProcessSignal.SIGTERM,
ProcessSignal.SIGINT,
];
/// Adds a signal handler to run on receipt of signal. /// Adds a signal handler to run on receipt of signal.
/// ///
...@@ -48,8 +45,17 @@ abstract class Signals { ...@@ -48,8 +45,17 @@ abstract class Signals {
Stream<Object> get errors; Stream<Object> get errors;
} }
class _DefaultSignals implements Signals { /// A class that manages the real dart:io signal handlers.
_DefaultSignals._(this.exitSignals); ///
/// We use a singleton instance of this class to ensure that all handlers for
/// fatal signals run before this class calls exit().
class LocalSignals implements Signals {
LocalSignals._(this.exitSignals);
static LocalSignals _instance;
static LocalSignals get instance => _instance ??= LocalSignals._(
Signals.defaultExitSignals,
);
final List<ProcessSignal> exitSignals; final List<ProcessSignal> exitSignals;
...@@ -84,7 +90,13 @@ class _DefaultSignals implements Signals { ...@@ -84,7 +90,13 @@ class _DefaultSignals implements Signals {
// If we added the first one, then call signal.watch(), listen, and cache // If we added the first one, then call signal.watch(), listen, and cache
// the stream controller. // the stream controller.
if (_handlersList[signal].length == 1) { if (_handlersList[signal].length == 1) {
_streamSubscriptions[signal] = signal.watch().listen(_handleSignal); _streamSubscriptions[signal] = signal.watch().listen(
_handleSignal,
onError: (Object e) {
_handlersTable[signal].remove(token);
_handlersList[signal].remove(handler);
},
);
} }
return token; return token;
} }
...@@ -99,7 +111,10 @@ class _DefaultSignals implements Signals { ...@@ -99,7 +111,10 @@ class _DefaultSignals implements Signals {
if (!_handlersTable[signal].containsKey(token)) { if (!_handlersTable[signal].containsKey(token)) {
return false; return false;
} }
final SignalHandler handler = _handlersTable[signal][token]; final SignalHandler handler = _handlersTable[signal].remove(token);
if (handler == null) {
return false;
}
final bool removed = _handlersList[signal].remove(handler); final bool removed = _handlersList[signal].remove(handler);
if (!removed) { if (!removed) {
return false; return false;
......
...@@ -103,41 +103,50 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -103,41 +103,50 @@ class AnalyzeOnce extends AnalyzeBase {
experiments: experiments, experiments: experiments,
); );
StreamSubscription<bool> subscription; Stopwatch timer;
subscription = server.onAnalyzing.listen((bool isAnalyzing) { Status progress;
if (!isAnalyzing) { try {
analysisCompleter.complete(); StreamSubscription<bool> subscription;
subscription?.cancel(); subscription = server.onAnalyzing.listen((bool isAnalyzing) {
subscription = null; if (!isAnalyzing) {
} analysisCompleter.complete();
}); subscription?.cancel();
server.onErrors.listen((FileAnalysisErrors fileErrors) { subscription = null;
// Record the issues found (but filter out to do comments). }
errors.addAll(fileErrors.errors.where((AnalysisError error) => error.type != 'TODO')); });
}); server.onErrors.listen((FileAnalysisErrors fileErrors) {
// Record the issues found (but filter out to do comments).
await server.start(); errors.addAll(fileErrors.errors.where((AnalysisError error) => error.type != 'TODO'));
// Completing the future in the callback can't fail. });
unawaited(server.onExit.then<void>((int exitCode) {
if (!analysisCompleter.isCompleted) { await server.start();
analysisCompleter.completeError('analysis server exited: $exitCode'); // Completing the future in the callback can't fail.
} unawaited(server.onExit.then<void>((int exitCode) {
})); if (!analysisCompleter.isCompleted) {
analysisCompleter.completeError('analysis server exited: $exitCode');
Cache.releaseLockEarly(); }
}));
// collect results
final Stopwatch timer = Stopwatch()..start(); Cache.releaseLockEarly();
final String message = directories.length > 1
? '${directories.length} ${directories.length == 1 ? 'directory' : 'directories'}' // collect results
: fileSystem.path.basename(directories.first); timer = Stopwatch()..start();
final Status progress = argResults['preamble'] as bool final String message = directories.length > 1
? logger.startProgress('Analyzing $message...', timeout: timeoutConfiguration.slowOperation) ? '${directories.length} ${directories.length == 1 ? 'directory' : 'directories'}'
: null; : fileSystem.path.basename(directories.first);
progress = argResults['preamble'] as bool
await analysisCompleter.future; ? logger.startProgress(
progress?.cancel(); 'Analyzing $message...',
timer.stop(); timeout: timeoutConfiguration.slowOperation,
)
: null;
await analysisCompleter.future;
} finally {
await server.dispose();
progress?.cancel();
timer?.stop();
}
// count missing dartdocs // count missing dartdocs
final int undocumentedMembers = errors.where((AnalysisError error) { final int undocumentedMembers = errors.where((AnalysisError error) {
......
...@@ -17,7 +17,6 @@ import 'base/io.dart'; ...@@ -17,7 +17,6 @@ import 'base/io.dart';
import 'base/logger.dart'; import 'base/logger.dart';
import 'base/os.dart'; import 'base/os.dart';
import 'base/process.dart'; import 'base/process.dart';
import 'base/signals.dart';
import 'base/time.dart'; import 'base/time.dart';
import 'base/user_messages.dart'; import 'base/user_messages.dart';
import 'build_system/build_system.dart'; import 'build_system/build_system.dart';
...@@ -182,7 +181,6 @@ Future<T> runInContext<T>( ...@@ -182,7 +181,6 @@ Future<T> runInContext<T>(
usage: globals.flutterUsage, usage: globals.flutterUsage,
), ),
ShutdownHooks: () => ShutdownHooks(logger: globals.logger), ShutdownHooks: () => ShutdownHooks(logger: globals.logger),
Signals: () => Signals(),
Stdio: () => Stdio(), Stdio: () => Stdio(),
SystemClock: () => const SystemClock(), SystemClock: () => const SystemClock(),
TimeoutConfiguration: () => const TimeoutConfiguration(), TimeoutConfiguration: () => const TimeoutConfiguration(),
......
...@@ -17,6 +17,7 @@ import 'base/logger.dart'; ...@@ -17,6 +17,7 @@ import 'base/logger.dart';
import 'base/net.dart'; import 'base/net.dart';
import 'base/os.dart'; import 'base/os.dart';
import 'base/platform.dart'; import 'base/platform.dart';
import 'base/signals.dart';
import 'base/template.dart'; import 'base/template.dart';
import 'base/terminal.dart'; import 'base/terminal.dart';
import 'base/user_messages.dart'; import 'base/user_messages.dart';
...@@ -45,20 +46,22 @@ HttpClientFactory get httpClientFactory => context.get<HttpClientFactory>(); ...@@ -45,20 +46,22 @@ HttpClientFactory get httpClientFactory => context.get<HttpClientFactory>();
Logger get logger => context.get<Logger>(); Logger get logger => context.get<Logger>();
OperatingSystemUtils get os => context.get<OperatingSystemUtils>(); OperatingSystemUtils get os => context.get<OperatingSystemUtils>();
PersistentToolState get persistentToolState => PersistentToolState.instance; PersistentToolState get persistentToolState => PersistentToolState.instance;
Signals get signals => context.get<Signals>() ?? LocalSignals.instance;
Usage get flutterUsage => context.get<Usage>(); Usage get flutterUsage => context.get<Usage>();
FlutterProjectFactory get projectFactory => context.get<FlutterProjectFactory>() ?? FlutterProjectFactory(
logger: logger,
fileSystem: fs,
);
const FileSystem _kLocalFs = LocalFileSystem(); FlutterProjectFactory get projectFactory {
return context.get<FlutterProjectFactory>() ?? FlutterProjectFactory(
logger: logger,
fileSystem: fs,
);
}
/// Currently active implementation of the file system. /// Currently active implementation of the file system.
/// ///
/// By default it uses local disk-based implementation. Override this in tests /// By default it uses local disk-based implementation. Override this in tests
/// with [MemoryFileSystem]. /// with [MemoryFileSystem].
FileSystem get fs => ErrorHandlingFileSystem( FileSystem get fs => ErrorHandlingFileSystem(
delegate: context.get<FileSystem>() ?? _kLocalFs, delegate: context.get<FileSystem>() ?? LocalFileSystem.instance,
platform: platform, platform: platform,
); );
......
...@@ -1295,7 +1295,7 @@ class TerminalHandler { ...@@ -1295,7 +1295,7 @@ class TerminalHandler {
final Map<io.ProcessSignal, Object> _signalTokens = <io.ProcessSignal, Object>{}; final Map<io.ProcessSignal, Object> _signalTokens = <io.ProcessSignal, Object>{};
void _addSignalHandler(io.ProcessSignal signal, SignalHandler handler) { void _addSignalHandler(io.ProcessSignal signal, SignalHandler handler) {
_signalTokens[signal] = signals.addHandler(signal, handler); _signalTokens[signal] = globals.signals.addHandler(signal, handler);
} }
void registerSignalHandlers() { void registerSignalHandlers() {
...@@ -1314,7 +1314,7 @@ class TerminalHandler { ...@@ -1314,7 +1314,7 @@ class TerminalHandler {
void stop() { void stop() {
assert(residentRunner.stayResident); assert(residentRunner.stayResident);
for (final MapEntry<io.ProcessSignal, Object> entry in _signalTokens.entries) { for (final MapEntry<io.ProcessSignal, Object> entry in _signalTokens.entries) {
signals.removeHandler(entry.key, entry.value); globals.signals.removeHandler(entry.key, entry.value);
} }
_signalTokens.clear(); _signalTokens.clear();
subscription.cancel(); subscription.cancel();
......
...@@ -712,8 +712,8 @@ abstract class FlutterCommand extends Command<void> { ...@@ -712,8 +712,8 @@ abstract class FlutterCommand extends Command<void> {
systemClock.now(), systemClock.now(),
); );
}; };
signals.addHandler(io.ProcessSignal.SIGTERM, handler); globals.signals.addHandler(io.ProcessSignal.SIGTERM, handler);
signals.addHandler(io.ProcessSignal.SIGINT, handler); globals.signals.addHandler(io.ProcessSignal.SIGINT, handler);
} }
/// Logs data about this command. /// Logs data about this command.
......
...@@ -31,7 +31,7 @@ void main() { ...@@ -31,7 +31,7 @@ void main() {
setUp(() { setUp(() {
platform = const LocalPlatform(); platform = const LocalPlatform();
fileSystem = const LocalFileSystem(); fileSystem = LocalFileSystem.instance;
platform = const LocalPlatform(); platform = const LocalPlatform();
processManager = const LocalProcessManager(); processManager = const LocalProcessManager();
terminal = AnsiTerminal(platform: platform, stdio: Stdio()); terminal = AnsiTerminal(platform: platform, stdio: Stdio());
......
...@@ -76,37 +76,39 @@ sky_engine:$flutterRootUri/bin/cache/pkg/sky_engine/lib/ ...@@ -76,37 +76,39 @@ sky_engine:$flutterRootUri/bin/cache/pkg/sky_engine/lib/
flutter_project:lib/ flutter_project:lib/
'''; ''';
fileSystem.file(fileSystem.path.join(projectPath, '.packages')) fileSystem.file(fileSystem.path.join(projectPath, '.packages'))
..createSync(recursive: true) ..createSync(recursive: true)
..writeAsStringSync(dotPackagesSrc); ..writeAsStringSync(dotPackagesSrc);
} }
setUpAll(() { setUpAll(() {
Cache.disableLocking(); Cache.disableLocking();
Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot; Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot;
processManager = const LocalProcessManager(); processManager = const LocalProcessManager();
terminal = AnsiTerminal(platform: platform, stdio: Stdio());
fileSystem = const LocalFileSystem();
platform = const LocalPlatform(); platform = const LocalPlatform();
terminal = AnsiTerminal(platform: platform, stdio: Stdio());
fileSystem = LocalFileSystem.instance;
logger = BufferLogger( logger = BufferLogger(
outputPreferences: OutputPreferences.test(), outputPreferences: OutputPreferences.test(),
terminal: terminal, terminal: terminal,
); );
analyzerSeparator = platform.isWindows ? '-' : '•'; analyzerSeparator = platform.isWindows ? '-' : '•';
tempDir = fileSystem.systemTempDirectory.createTempSync('flutter_analyze_once_test_1.').absolute; });
setUp(() {
tempDir = fileSystem.systemTempDirectory.createTempSync(
'flutter_analyze_once_test_1.',
).absolute;
projectPath = fileSystem.path.join(tempDir.path, 'flutter_project'); projectPath = fileSystem.path.join(tempDir.path, 'flutter_project');
fileSystem.file(fileSystem.path.join(projectPath, 'pubspec.yaml')) fileSystem.file(fileSystem.path.join(projectPath, 'pubspec.yaml'))
..createSync(recursive: true) ..createSync(recursive: true)
..writeAsStringSync(pubspecYamlSrc); ..writeAsStringSync(pubspecYamlSrc);
_createDotPackages(projectPath); _createDotPackages(projectPath);
});
setUp(() {
libMain = fileSystem.file(fileSystem.path.join(projectPath, 'lib', 'main.dart')) libMain = fileSystem.file(fileSystem.path.join(projectPath, 'lib', 'main.dart'))
..createSync(recursive: true) ..createSync(recursive: true)
..writeAsStringSync(mainDartSrc); ..writeAsStringSync(mainDartSrc);
}); });
tearDownAll(() { tearDown(() {
tryToDelete(tempDir); tryToDelete(tempDir);
}); });
......
...@@ -226,7 +226,7 @@ void main() { ...@@ -226,7 +226,7 @@ void main() {
testWithoutContext('Throws type error if Directory type is set to curentDirectory with LocalFileSystem', () { testWithoutContext('Throws type error if Directory type is set to curentDirectory with LocalFileSystem', () {
final FileSystem fs = ErrorHandlingFileSystem( final FileSystem fs = ErrorHandlingFileSystem(
delegate: const LocalFileSystem(), delegate: LocalFileSystem.instance,
platform: const LocalPlatform(), platform: const LocalPlatform(),
); );
final MockDirectory directory = MockDirectory(); final MockDirectory directory = MockDirectory();
......
...@@ -2,12 +2,18 @@ ...@@ -2,12 +2,18 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'dart:io' as io;
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/context.dart';
class MockPlatform extends Mock implements Platform {} class MockPlatform extends Mock implements Platform {}
...@@ -121,4 +127,40 @@ void main() { ...@@ -121,4 +127,40 @@ void main() {
expect(fsUtils.escapePath(r'foo\cool.dart'), r'foo\cool.dart'); expect(fsUtils.escapePath(r'foo\cool.dart'), r'foo\cool.dart');
}); });
}); });
group('LocalFileSystem', () {
MockIoProcessSignal mockSignal;
ProcessSignal signalUnderTest;
StreamController<io.ProcessSignal> controller;
setUp(() {
mockSignal = MockIoProcessSignal();
signalUnderTest = ProcessSignal(mockSignal);
controller = StreamController<io.ProcessSignal>();
when(mockSignal.watch()).thenAnswer((Invocation invocation) => controller.stream);
});
testUsingContext('deletes system temp entry on a fatal signal', () async {
final Completer<void> completer = Completer<void>();
final Signals signals = Signals.test();
final LocalFileSystem localFileSystem = LocalFileSystem.test(
signals: signals,
fatalSignals: <ProcessSignal>[signalUnderTest],
);
final Directory temp = localFileSystem.systemTempDirectory;
signals.addHandler(signalUnderTest, (ProcessSignal s) {
completer.complete();
});
expect(temp.existsSync(), isTrue);
controller.add(mockSignal);
await completer.future;
expect(temp.existsSync(), isFalse);
});
});
} }
class MockIoProcessSignal extends Mock implements io.ProcessSignal {}
...@@ -10,22 +10,23 @@ import 'package:flutter_tools/src/base/signals.dart'; ...@@ -10,22 +10,23 @@ import 'package:flutter_tools/src/base/signals.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/context.dart';
void main() { void main() {
group('Signals', () { group('Signals', () {
Signals signals;
MockIoProcessSignal mockSignal; MockIoProcessSignal mockSignal;
ProcessSignal signalUnderTest; ProcessSignal signalUnderTest;
StreamController<io.ProcessSignal> controller; StreamController<io.ProcessSignal> controller;
setUp(() { setUp(() {
signals = Signals.test();
mockSignal = MockIoProcessSignal(); mockSignal = MockIoProcessSignal();
signalUnderTest = ProcessSignal(mockSignal); signalUnderTest = ProcessSignal(mockSignal);
controller = StreamController<io.ProcessSignal>(); controller = StreamController<io.ProcessSignal>();
when(mockSignal.watch()).thenAnswer((Invocation invocation) => controller.stream); when(mockSignal.watch()).thenAnswer((Invocation invocation) => controller.stream);
}); });
testUsingContext('signal handler runs', () async { testWithoutContext('signal handler runs', () async {
final Completer<void> completer = Completer<void>(); final Completer<void> completer = Completer<void>();
signals.addHandler(signalUnderTest, (ProcessSignal s) { signals.addHandler(signalUnderTest, (ProcessSignal s) {
expect(s, signalUnderTest); expect(s, signalUnderTest);
...@@ -34,11 +35,9 @@ void main() { ...@@ -34,11 +35,9 @@ void main() {
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('signal handlers run in order', () async { testWithoutContext('signal handlers run in order', () async {
final Completer<void> completer = Completer<void>(); final Completer<void> completer = Completer<void>();
bool first = false; bool first = false;
...@@ -56,11 +55,9 @@ void main() { ...@@ -56,11 +55,9 @@ void main() {
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('signal handler error goes on error stream', () async { testWithoutContext('signal handler error goes on error stream', () async {
final Exception exn = Exception('Error'); final Exception exn = Exception('Error');
signals.addHandler(signalUnderTest, (ProcessSignal s) { signals.addHandler(signalUnderTest, (ProcessSignal s) {
throw exn; throw exn;
...@@ -68,65 +65,71 @@ void main() { ...@@ -68,65 +65,71 @@ void main() {
final Completer<void> completer = Completer<void>(); final Completer<void> completer = Completer<void>();
final List<Object> errList = <Object>[]; final List<Object> errList = <Object>[];
final StreamSubscription<Object> errSub = signals.errors.listen((Object err) { final StreamSubscription<Object> errSub = signals.errors.listen(
errList.add(err); (Object err) {
completer.complete(); errList.add(err);
}); completer.complete();
},
);
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
await errSub.cancel(); await errSub.cancel();
expect(errList, contains(exn)); expect(errList, contains(exn));
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('removed signal handler does not run', () async { testWithoutContext('removed signal handler does not run', () async {
final Object token = signals.addHandler(signalUnderTest, (ProcessSignal s) { final Object token = signals.addHandler(
fail('Signal handler should have been removed.'); signalUnderTest,
}); (ProcessSignal s) {
fail('Signal handler should have been removed.');
},
);
await signals.removeHandler(signalUnderTest, token); await signals.removeHandler(signalUnderTest, token);
final List<Object> errList = <Object>[]; final List<Object> errList = <Object>[];
final StreamSubscription<Object> errSub = signals.errors.listen((Object err) { final StreamSubscription<Object> errSub = signals.errors.listen(
errList.add(err); (Object err) {
}); errList.add(err);
},
);
controller.add(mockSignal); controller.add(mockSignal);
await errSub.cancel(); await errSub.cancel();
expect(errList, isEmpty); expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('non-removed signal handler still runs', () async { testWithoutContext('non-removed signal handler still runs', () async {
final Completer<void> completer = Completer<void>(); final Completer<void> completer = Completer<void>();
signals.addHandler(signalUnderTest, (ProcessSignal s) { signals.addHandler(signalUnderTest, (ProcessSignal s) {
expect(s, signalUnderTest); expect(s, signalUnderTest);
completer.complete(); completer.complete();
}); });
final Object token = signals.addHandler(signalUnderTest, (ProcessSignal s) { final Object token = signals.addHandler(
fail('Signal handler should have been removed.'); signalUnderTest,
}); (ProcessSignal s) {
fail('Signal handler should have been removed.');
},
);
await signals.removeHandler(signalUnderTest, token); await signals.removeHandler(signalUnderTest, token);
final List<Object> errList = <Object>[]; final List<Object> errList = <Object>[];
final StreamSubscription<Object> errSub = signals.errors.listen((Object err) { final StreamSubscription<Object> errSub = signals.errors.listen(
errList.add(err); (Object err) {
}); errList.add(err);
},
);
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
await errSub.cancel(); await errSub.cancel();
expect(errList, isEmpty); expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('only handlers for the correct signal run', () async { testWithoutContext('only handlers for the correct signal run', () async {
final MockIoProcessSignal mockSignal2 = MockIoProcessSignal(); final MockIoProcessSignal mockSignal2 = MockIoProcessSignal();
final StreamController<io.ProcessSignal> controller2 = StreamController<io.ProcessSignal>(); final StreamController<io.ProcessSignal> controller2 = StreamController<io.ProcessSignal>();
final ProcessSignal otherSignal = ProcessSignal(mockSignal2); final ProcessSignal otherSignal = ProcessSignal(mockSignal2);
...@@ -144,19 +147,22 @@ void main() { ...@@ -144,19 +147,22 @@ void main() {
}); });
final List<Object> errList = <Object>[]; final List<Object> errList = <Object>[];
final StreamSubscription<Object> errSub = signals.errors.listen((Object err) { final StreamSubscription<Object> errSub = signals.errors.listen(
errList.add(err); (Object err) {
}); errList.add(err);
},
);
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
await errSub.cancel(); await errSub.cancel();
expect(errList, isEmpty); expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
}); });
testUsingContext('all handlers for exiting signals are run before exit', () async { testWithoutContext('all handlers for exiting signals are run before exit', () async {
final Signals signals = Signals.test(
exitSignals: <ProcessSignal>[signalUnderTest],
);
final Completer<void> completer = Completer<void>(); final Completer<void> completer = Completer<void>();
bool first = false; bool first = false;
bool second = false; bool second = false;
...@@ -186,8 +192,6 @@ void main() { ...@@ -186,8 +192,6 @@ void main() {
controller.add(mockSignal); controller.add(mockSignal);
await completer.future; await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(exitSignals: <ProcessSignal>[signalUnderTest]),
}); });
}); });
} }
......
...@@ -53,6 +53,7 @@ void main() { ...@@ -53,6 +53,7 @@ void main() {
setUp(() { setUp(() {
mockPlatform = MockPlatform(); mockPlatform = MockPlatform();
when(mockPlatform.isWindows).thenReturn(false);
mockProcessManager = MockProcessManager(); mockProcessManager = MockProcessManager();
flutterPlatform = TestFlutterPlatform(); flutterPlatform = TestFlutterPlatform();
}); });
......
...@@ -134,6 +134,27 @@ void main() { ...@@ -134,6 +134,27 @@ void main() {
} }
}); });
test('no unauthorized imports of package:file/local.dart', () {
final List<String> whitelistedPath = <String>[
globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'file_system.dart'),
];
for (final String dirName in <String>['lib', 'bin', 'test']) {
final Iterable<File> files = globals.fs.directory(globals.fs.path.join(flutterTools, dirName))
.listSync(recursive: true)
.where(_isDartFile)
.where((FileSystemEntity entity) => !whitelistedPath.contains(entity.path))
.map(_asFile);
for (final File file in files) {
for (final String line in file.readAsLinesSync()) {
if (line.startsWith(RegExp(r'import.*package:file/local.dart'))) {
final String relativePath = globals.fs.path.relative(file.path, from:flutterTools);
fail("$relativePath imports 'package:file/local.dart'; use 'lib/src/base/file_system.dart' instead");
}
}
}
}
});
test('no unauthorized imports of dart:convert', () { test('no unauthorized imports of dart:convert', () {
final List<String> whitelistedPaths = <String>[ final List<String> whitelistedPaths = <String>[
globals.fs.path.join(flutterTools, 'lib', 'src', 'convert.dart'), globals.fs.path.join(flutterTools, 'lib', 'src', 'convert.dart'),
......
...@@ -52,7 +52,7 @@ void main() { ...@@ -52,7 +52,7 @@ void main() {
stdio: null, stdio: null,
), ),
); );
fileSystem = const LocalFileSystemBlockingSetCurrentDirectory(); fileSystem = LocalFileSystemBlockingSetCurrentDirectory();
processManager = const LocalProcessManager(); processManager = const LocalProcessManager();
parser = PlistParser( parser = PlistParser(
fileSystem: fileSystem, fileSystem: fileSystem,
......
...@@ -450,7 +450,7 @@ class FakeSignals implements Signals { ...@@ -450,7 +450,7 @@ class FakeSignals implements Signals {
FakeSignals({ FakeSignals({
this.subForSigTerm, this.subForSigTerm,
List<ProcessSignal> exitSignals, List<ProcessSignal> exitSignals,
}) : delegate = Signals(exitSignals: exitSignals); }) : delegate = Signals.test(exitSignals: exitSignals);
final ProcessSignal subForSigTerm; final ProcessSignal subForSigTerm;
final Signals delegate; final Signals delegate;
......
...@@ -14,7 +14,7 @@ import '../src/common.dart'; ...@@ -14,7 +14,7 @@ import '../src/common.dart';
const String _kInitialVersion = 'v1.9.1'; const String _kInitialVersion = 'v1.9.1';
const String _kBranch = 'dev'; const String _kBranch = 'dev';
const FileSystem fileSystem = LocalFileSystem(); final FileSystem fileSystem = LocalFileSystem.instance;
const ProcessManager processManager = LocalProcessManager(); const ProcessManager processManager = LocalProcessManager();
final Stdio stdio = Stdio(); final Stdio stdio = Stdio();
final ProcessUtils processUtils = ProcessUtils(processManager: processManager, logger: StdoutLogger( final ProcessUtils processUtils = ProcessUtils(processManager: processManager, logger: StdoutLogger(
......
...@@ -21,7 +21,7 @@ void main() { ...@@ -21,7 +21,7 @@ void main() {
FlutterRunTestDriver flutter; FlutterRunTestDriver flutter;
VmService vmService; VmService vmService;
setUpAll(() async { setUp(() async {
tempDir = createResolvedTempDirectorySync('vmservice_integration_test.'); tempDir = createResolvedTempDirectorySync('vmservice_integration_test.');
final BasicProject _project = BasicProject(); final BasicProject _project = BasicProject();
...@@ -33,7 +33,7 @@ void main() { ...@@ -33,7 +33,7 @@ void main() {
vmService = await vmServiceConnectUri('ws://localhost:$port/ws'); vmService = await vmServiceConnectUri('ws://localhost:$port/ws');
}); });
tearDownAll(() async { tearDown(() async {
await flutter?.stop(); await flutter?.stop();
tryToDelete(tempDir); tryToDelete(tempDir);
}); });
......
...@@ -20,7 +20,7 @@ import 'package:meta/meta.dart'; ...@@ -20,7 +20,7 @@ import 'package:meta/meta.dart';
import 'package:test_api/test_api.dart' as test_package show TypeMatcher, test; // ignore: deprecated_member_use import 'package:test_api/test_api.dart' as test_package show TypeMatcher, test; // ignore: deprecated_member_use
import 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; // ignore: deprecated_member_use import 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; // ignore: deprecated_member_use
// ignore: deprecated_member_use // ignore: deprecated_member_use
export 'package:test_core/test_core.dart' hide TypeMatcher, isInstanceOf; // Defines a 'package:test' shim. export 'package:test_core/test_core.dart' hide TypeMatcher, isInstanceOf, test; // Defines a 'package:test' shim.
/// A matcher that compares the type of the actual value to the type argument T. /// A matcher that compares the type of the actual value to the type argument T.
// TODO(ianh): Remove this once https://github.com/dart-lang/matcher/issues/98 is fixed // TODO(ianh): Remove this once https://github.com/dart-lang/matcher/issues/98 is fixed
...@@ -31,7 +31,9 @@ void tryToDelete(Directory directory) { ...@@ -31,7 +31,9 @@ void tryToDelete(Directory directory) {
// on Windows it's common for deletions to fail due to // on Windows it's common for deletions to fail due to
// bogus (we think) "access denied" errors. // bogus (we think) "access denied" errors.
try { try {
directory.deleteSync(recursive: true); if (directory.existsSync()) {
directory.deleteSync(recursive: true);
}
} on FileSystemException catch (error) { } on FileSystemException catch (error) {
print('Failed to delete ${directory.path}: $error'); print('Failed to delete ${directory.path}: $error');
} }
...@@ -156,6 +158,35 @@ Matcher containsIgnoringWhitespace(String toSearch) { ...@@ -156,6 +158,35 @@ Matcher containsIgnoringWhitespace(String toSearch) {
); );
} }
/// The tool overrides `test` to ensure that files created under the
/// system temporary directory are deleted after each test by calling
/// `LocalFileSystem.dispose()`.
@isTest
void test(String description, FutureOr<void> body(), {
String testOn,
Timeout timeout,
dynamic skip,
List<String> tags,
Map<String, dynamic> onPlatform,
int retry,
}) {
test_package.test(
description,
() async {
addTearDown(() async {
await LocalFileSystem.dispose();
});
return body();
},
timeout: timeout,
skip: skip,
tags: tags,
onPlatform: onPlatform,
retry: retry,
testOn: testOn,
);
}
/// Executes a test body in zone that does not allow context-based injection. /// Executes a test body in zone that does not allow context-based injection.
/// ///
/// For classes which have been refactored to excluded context-based injection /// For classes which have been refactored to excluded context-based injection
...@@ -168,12 +199,12 @@ Matcher containsIgnoringWhitespace(String toSearch) { ...@@ -168,12 +199,12 @@ Matcher containsIgnoringWhitespace(String toSearch) {
void testWithoutContext(String description, FutureOr<void> body(), { void testWithoutContext(String description, FutureOr<void> body(), {
String testOn, String testOn,
Timeout timeout, Timeout timeout,
bool skip, dynamic skip,
List<String> tags, List<String> tags,
Map<String, dynamic> onPlatform, Map<String, dynamic> onPlatform,
int retry, int retry,
}) { }) {
return test_package.test( return test(
description, () async { description, () async {
return runZoned(body, zoneValues: <Object, Object>{ return runZoned(body, zoneValues: <Object, Object>{
contextKey: const NoContext(), contextKey: const NoContext(),
......
...@@ -126,7 +126,7 @@ void testUsingContext( ...@@ -126,7 +126,7 @@ void testUsingContext(
SimControl: () => MockSimControl(), SimControl: () => MockSimControl(),
Usage: () => FakeUsage(), Usage: () => FakeUsage(),
XcodeProjectInterpreter: () => FakeXcodeProjectInterpreter(), XcodeProjectInterpreter: () => FakeXcodeProjectInterpreter(),
FileSystem: () => const LocalFileSystemBlockingSetCurrentDirectory(), FileSystem: () => LocalFileSystemBlockingSetCurrentDirectory(),
TimeoutConfiguration: () => const TimeoutConfiguration(), TimeoutConfiguration: () => const TimeoutConfiguration(),
PlistParser: () => FakePlistParser(), PlistParser: () => FakePlistParser(),
Signals: () => FakeSignals(), Signals: () => FakeSignals(),
...@@ -443,7 +443,9 @@ class FakePlistParser implements PlistParser { ...@@ -443,7 +443,9 @@ class FakePlistParser implements PlistParser {
} }
class LocalFileSystemBlockingSetCurrentDirectory extends LocalFileSystem { class LocalFileSystemBlockingSetCurrentDirectory extends LocalFileSystem {
const LocalFileSystemBlockingSetCurrentDirectory(); LocalFileSystemBlockingSetCurrentDirectory() : super.test(
signals: LocalSignals.instance,
);
@override @override
set currentDirectory(dynamic value) { set currentDirectory(dynamic value) {
......
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