Skip to content

Commit cb22250

Browse files
srawlinssrawlins
authored
Improve temp test dir cleanup code (#10025)
1 parent ebbbda1 commit cb22250

9 files changed

Lines changed: 84 additions & 74 deletions

File tree

‎packages/devtools_app/test/shared/diagnostics/inspector_service_test.dart‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
@TestOn('vm')
88
library;
99

10-
import 'dart:async';
11-
1210
import 'package:devtools_app/devtools_app.dart';
1311
import 'package:devtools_app_shared/shared.dart';
1412
import 'package:devtools_app_shared/ui.dart';
@@ -47,7 +45,10 @@ void main() {
4745
try {
4846
group('inspector service tests', () {
4947
tearDown(env.tearDownEnvironment);
50-
tearDownAll(() => unawaited(env.tearDownEnvironment(force: true)));
48+
tearDownAll(() async {
49+
await env.tearDownEnvironment(force: true);
50+
env.finalTeardown();
51+
});
5152

5253
test('track widget creation on', () async {
5354
await env.setupEnvironment();

‎packages/devtools_app/test/test_infra/flutter_test_environment.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,12 @@ class FlutterTestEnvironment {
200200
_needsSetup = true;
201201
}
202202

203+
/// Deletes the temporary directory created for the test suite.
203204
void finalTeardown() {
204-
// Delete the temporary directory created for the test suite.
205-
if (_tempTestAppDirectory != null) {
206-
final tempDirectory = Directory(_tempTestAppDirectory!);
205+
if (_tempTestAppDirectory case final tempTestAppDirectory?) {
206+
final tempDirectory = Directory(tempTestAppDirectory);
207207
if (tempDirectory.existsSync()) {
208-
Directory(_tempTestAppDirectory!).deleteSync(recursive: true);
208+
tempDirectory.deleteSync(recursive: true);
209209
}
210210
}
211211
}

‎packages/devtools_shared/test/deeplink/deeplink_manager_test.dart‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ void main() {
2020
});
2121

2222
tearDown(() {
23-
expect(
24-
manager.expectedCommands.isEmpty,
25-
true,
26-
reason:
27-
'stub does not receive expected command ${manager.expectedCommands}',
28-
);
29-
tmpDir.deleteSync(recursive: true);
23+
try {
24+
expect(
25+
manager.expectedCommands.isEmpty,
26+
true,
27+
reason:
28+
'stub does not receive expected command ${manager.expectedCommands}',
29+
);
30+
} finally {
31+
tmpDir.deleteSync(recursive: true);
32+
}
3033
});
3134

3235
test('getBuildVariants calls flutter command correctly', () async {

‎packages/devtools_shared/test/helpers/extension_test_manager.dart‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ class ExtensionTestManager {
2828

2929
Future<void> reset() async {
3030
// Run with retry to ensure this deletes properly on Windows.
31-
await deleteDirectoryWithRetry(testDirectory);
32-
_testDirectory = null;
31+
if (_testDirectory case final testDirectory?) {
32+
await deleteDirectoryWithRetry(testDirectory);
33+
_testDirectory = null;
34+
}
3335
_packagesRootUri = null;
3436
_runtimeAppRoot = null;
3537
}

‎packages/devtools_shared/test/helpers/helpers.dart‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ Future<TestDtdConnectionInfo> startDtd() async {
6363

6464
class TestDartApp {
6565
TestDartApp() {
66-
directory = Directory(
67-
'tmp/test_app_${DateTime.now().millisecondsSinceEpoch}',
68-
);
66+
directory = Directory.systemTemp.createTempSync('test_app_');
6967
}
7068
static final dartVMServiceRegExp = RegExp(
7169
r'The Dart VM service is listening on (http://127.0.0.1:.*)',
@@ -78,7 +76,7 @@ class TestDartApp {
7876
StreamSubscription<String>? _stderrSub;
7977

8078
Future<String> start() async {
81-
await _initTestApp();
79+
_initTestApp();
8280
process = await Process.start(Platform.resolvedExecutable, [
8381
'--observe=0',
8482
'bin/main.dart',
@@ -122,10 +120,10 @@ class TestDartApp {
122120
await deleteDirectoryWithRetry(directory);
123121
}
124122

125-
Future<void> _initTestApp() async {
126-
await deleteDirectoryWithRetry(directory);
127-
directory.createSync(recursive: true);
128-
Directory(path.join(directory.path, '.dart_tool')).createSync();
123+
void _initTestApp() {
124+
Directory(
125+
path.join(directory.path, '.dart_tool'),
126+
).createSync(recursive: true);
129127

130128
final mainFile = File(path.join(directory.path, 'bin', 'main.dart'))
131129
..createSync(recursive: true);

‎packages/devtools_shared/test/server/general_api_test.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ void main() {
186186
expect(response.success, true);
187187
expect(response.message, isNull);
188188
expect(response.uri, isNotNull);
189-
expect(response.uri!.toString(), endsWith(app!.directory.path));
189+
expect(response.uri!.toFilePath(), endsWith(app!.directory.path));
190190
});
191191

192192
test('succeeds for a disconnect event when cache is empty', () async {
@@ -223,7 +223,7 @@ void main() {
223223
expect(response.success, true);
224224
expect(response.message, isNull);
225225
expect(response.uri, isNotNull);
226-
expect(response.uri!.toString(), endsWith(app!.directory.path));
226+
expect(response.uri!.toFilePath(), endsWith(app!.directory.path));
227227

228228
final disconnectResponse =
229229
await server.VmServiceHandler.detectRootPackageForVmService(
@@ -236,7 +236,7 @@ void main() {
236236
expect(disconnectResponse.message, isNull);
237237
expect(disconnectResponse.uri, isNotNull);
238238
expect(
239-
disconnectResponse.uri!.toString(),
239+
disconnectResponse.uri!.toFilePath(),
240240
endsWith(app!.directory.path),
241241
);
242242
},

‎packages/devtools_shared/test/utils/file_utils_test.dart‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ void main() {
5656
dtd?.process?.kill();
5757
await dtd?.process?.exitCode;
5858
dtd = null;
59-
});
6059

61-
tearDownAll(() async {
6260
// Run with retry to ensure this deletes properly on Windows.
6361
await deleteDirectoryWithRetry(testDirectory);
6462
});

‎tool/lib/commands/rollback.dart‎

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -30,51 +30,59 @@ class RollbackCommand extends Command {
3030
final tempDir = (await io.Directory.systemTemp.createTemp(
3131
'devtools-rollback',
3232
)).absolute;
33-
print('file://${tempDir.path}');
34-
final tarball = io.File('${tempDir.path}/devtools.tar.gz');
35-
final extractDir = await io.Directory(
36-
'${tempDir.path}/extract/',
37-
).absolute.create();
38-
final client = io.HttpClient();
39-
final version = argResults![_toVersionArg] as String;
40-
print('downloading tarball to ${tarball.path}');
41-
final tarballRequest = await client.getUrl(
42-
Uri.http(
43-
'storage.googleapis.com',
44-
'pub-packages/packages/devtools-$version.tar.gz',
45-
),
46-
);
47-
final tarballResponse = await tarballRequest.close();
48-
await tarballResponse.pipe(tarball.openWrite());
49-
print('Tarball written; unzipping.');
33+
try {
34+
print('file://${tempDir.path}');
35+
final tarball = io.File('${tempDir.path}/devtools.tar.gz');
36+
final extractDir = await io.Directory(
37+
'${tempDir.path}/extract/',
38+
).absolute.create();
39+
final client = io.HttpClient();
40+
final version = argResults![_toVersionArg] as String;
41+
print('downloading tarball to ${tarball.path}');
42+
final tarballRequest = await client.getUrl(
43+
Uri.http(
44+
'storage.googleapis.com',
45+
'pub-packages/packages/devtools-$version.tar.gz',
46+
),
47+
);
48+
final tarballResponse = await tarballRequest.close();
49+
await tarballResponse.pipe(tarball.openWrite());
50+
print('Tarball written; unzipping.');
5051

51-
await io.Process.run('tar', [
52-
'-x',
53-
'-z',
54-
'-f',
55-
tarball.path.split('/').last,
56-
'-C',
57-
extractDir.path,
58-
], workingDirectory: tempDir.path);
59-
print('file://${tempDir.path}');
52+
await io.Process.run('tar', [
53+
'-x',
54+
'-z',
55+
'-f',
56+
tarball.path.split('/').last,
57+
'-C',
58+
extractDir.path,
59+
], workingDirectory: tempDir.path);
60+
print('file://${tempDir.path}');
6061

61-
final buildDir = io.Directory('${repo.repoPath}/packages/devtools/build/');
62-
await buildDir.delete(recursive: true);
63-
await io.Directory(
64-
'${extractDir.path}build/',
65-
).rename('${repo.repoPath}/packages/devtools/build/');
62+
final buildDir = io.Directory(
63+
'${repo.repoPath}/packages/devtools/build/',
64+
);
65+
await buildDir.delete(recursive: true);
66+
await io.Directory(
67+
'${extractDir.path}build/',
68+
).rename('${repo.repoPath}/packages/devtools/build/');
6669

67-
print(
68-
'Build outputs from Devtools version $version checked out and moved '
69-
'to ${buildDir.path}',
70-
);
71-
print(
72-
'To complete the rollback, go to ${repo.repoPath}/packages/devtools, '
73-
'rev pubspec.yaml, update the changelog, unhide build/ from the '
74-
'packages/devtools/.gitignore file, then run pub publish.',
75-
);
76-
// TODO(djshuckerow): automatically rev pubspec.yaml and update the
77-
// changelog so that the user can just run pub publish from
78-
// packages/devtools.
70+
print(
71+
'Build outputs from Devtools version $version checked out and moved '
72+
'to ${buildDir.path}',
73+
);
74+
print(
75+
'To complete the rollback, go to ${repo.repoPath}/packages/devtools, '
76+
'rev pubspec.yaml, update the changelog, unhide build/ from the '
77+
'packages/devtools/.gitignore file, then run pub publish.',
78+
);
79+
// TODO(djshuckerow): automatically rev pubspec.yaml and update the
80+
// changelog so that the user can just run pub publish from
81+
// packages/devtools.
82+
} finally {
83+
if (await tempDir.exists()) {
84+
await tempDir.delete(recursive: true);
85+
}
86+
}
7987
}
8088
}

‎tool/test/license_utils_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void main() {
6060
await _setupTestConfigFile();
6161
});
6262

63-
tearDownAll(() async {
63+
tearDown(() async {
6464
await deleteDirectoryWithRetry(testDirectory);
6565
});
6666

@@ -171,7 +171,7 @@ text that should be removed from the file. */
171171
await _setupTestDirectoryStructure();
172172
});
173173

174-
tearDownAll(() async {
174+
tearDown(() async {
175175
await deleteDirectoryWithRetry(testDirectory);
176176
});
177177

0 commit comments

Comments
 (0)