From d8770a15da1c66358ade06778b7149a0a44a9c28 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 29 Sep 2026 16:43:18 -0700 Subject: [PATCH 1/5] Improve temp test dir cleanup code --- .../diagnostics/inspector_service_test.dart | 7 +- .../test_infra/flutter_test_environment.dart | 10 +- .../test/deeplink/deeplink_manager_test.dart | 17 ++-- .../test/helpers/extension_test_manager.dart | 6 +- .../devtools_shared/test/helpers/helpers.dart | 10 +- .../test/utils/file_utils_test.dart | 2 - tool/lib/commands/rollback.dart | 94 ++++++++++--------- tool/test/license_utils_test.dart | 4 +- 8 files changed, 80 insertions(+), 70 deletions(-) diff --git a/packages/devtools_app/test/shared/diagnostics/inspector_service_test.dart b/packages/devtools_app/test/shared/diagnostics/inspector_service_test.dart index 99b29293730..655860a62a0 100644 --- a/packages/devtools_app/test/shared/diagnostics/inspector_service_test.dart +++ b/packages/devtools_app/test/shared/diagnostics/inspector_service_test.dart @@ -7,8 +7,6 @@ @TestOn('vm') library; -import 'dart:async'; - import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app_shared/shared.dart'; import 'package:devtools_app_shared/ui.dart'; @@ -47,7 +45,10 @@ void main() { try { group('inspector service tests', () { tearDown(env.tearDownEnvironment); - tearDownAll(() => unawaited(env.tearDownEnvironment(force: true))); + tearDownAll(() async { + await env.tearDownEnvironment(force: true); + env.finalTeardown(); + }); test('track widget creation on', () async { await env.setupEnvironment(); diff --git a/packages/devtools_app/test/test_infra/flutter_test_environment.dart b/packages/devtools_app/test/test_infra/flutter_test_environment.dart index 71906b77fb1..506775bbd80 100644 --- a/packages/devtools_app/test/test_infra/flutter_test_environment.dart +++ b/packages/devtools_app/test/test_infra/flutter_test_environment.dart @@ -200,12 +200,12 @@ class FlutterTestEnvironment { _needsSetup = true; } + /// Deletes the temporary directory created for the test suite. void finalTeardown() { - // Delete the temporary directory created for the test suite. - if (_tempTestAppDirectory != null) { - final tempDirectory = Directory(_tempTestAppDirectory!); - if (tempDirectory.existsSync()) { - Directory(_tempTestAppDirectory!).deleteSync(recursive: true); + if (_tempTestAppDirectory case final tempTestAppDirectory?) { + final tempDirectory = Directory(tempTestAppDirectory); + if (Directory(tempTestAppDirectory).existsSync()) { + tempDirectory.deleteSync(recursive: true); } } } diff --git a/packages/devtools_shared/test/deeplink/deeplink_manager_test.dart b/packages/devtools_shared/test/deeplink/deeplink_manager_test.dart index fb36b123e99..f45e0dbe141 100644 --- a/packages/devtools_shared/test/deeplink/deeplink_manager_test.dart +++ b/packages/devtools_shared/test/deeplink/deeplink_manager_test.dart @@ -19,13 +19,16 @@ void main() { }); tearDown(() { - expect( - manager.expectedCommands.isEmpty, - true, - reason: - 'stub does not receive expected command ${manager.expectedCommands}', - ); - tmpDir.deleteSync(recursive: true); + try { + expect( + manager.expectedCommands.isEmpty, + true, + reason: + 'stub does not receive expected command ${manager.expectedCommands}', + ); + } finally { + tmpDir.deleteSync(recursive: true); + } }); test('getBuildVariants calls flutter command correctly', () async { diff --git a/packages/devtools_shared/test/helpers/extension_test_manager.dart b/packages/devtools_shared/test/helpers/extension_test_manager.dart index 00add39d1ca..0ff7c63d55c 100644 --- a/packages/devtools_shared/test/helpers/extension_test_manager.dart +++ b/packages/devtools_shared/test/helpers/extension_test_manager.dart @@ -28,8 +28,10 @@ class ExtensionTestManager { Future reset() async { // Run with retry to ensure this deletes properly on Windows. - await deleteDirectoryWithRetry(testDirectory); - _testDirectory = null; + if (_testDirectory case final testDirectory?) { + await deleteDirectoryWithRetry(testDirectory); + _testDirectory = null; + } _packagesRootUri = null; _runtimeAppRoot = null; } diff --git a/packages/devtools_shared/test/helpers/helpers.dart b/packages/devtools_shared/test/helpers/helpers.dart index 8c03895945d..1235f957474 100644 --- a/packages/devtools_shared/test/helpers/helpers.dart +++ b/packages/devtools_shared/test/helpers/helpers.dart @@ -63,9 +63,7 @@ Future startDtd() async { class TestDartApp { TestDartApp() { - directory = Directory( - 'tmp/test_app_${DateTime.now().millisecondsSinceEpoch}', - ); + directory = Directory.systemTemp.createTempSync('test_app_'); } static final dartVMServiceRegExp = RegExp( r'The Dart VM service is listening on (http://127.0.0.1:.*)', @@ -123,9 +121,9 @@ class TestDartApp { } Future _initTestApp() async { - await deleteDirectoryWithRetry(directory); - directory.createSync(recursive: true); - Directory(path.join(directory.path, '.dart_tool')).createSync(); + Directory( + path.join(directory.path, '.dart_tool'), + ).createSync(recursive: true); final mainFile = File(path.join(directory.path, 'bin', 'main.dart')) ..createSync(recursive: true); diff --git a/packages/devtools_shared/test/utils/file_utils_test.dart b/packages/devtools_shared/test/utils/file_utils_test.dart index 03b6c9a523c..f8b5bdc2f0b 100644 --- a/packages/devtools_shared/test/utils/file_utils_test.dart +++ b/packages/devtools_shared/test/utils/file_utils_test.dart @@ -56,9 +56,7 @@ void main() { dtd?.process?.kill(); await dtd?.process?.exitCode; dtd = null; - }); - tearDownAll(() async { // Run with retry to ensure this deletes properly on Windows. await deleteDirectoryWithRetry(testDirectory); }); diff --git a/tool/lib/commands/rollback.dart b/tool/lib/commands/rollback.dart index de8c5e974b9..0a0680af60a 100644 --- a/tool/lib/commands/rollback.dart +++ b/tool/lib/commands/rollback.dart @@ -30,51 +30,59 @@ class RollbackCommand extends Command { final tempDir = (await io.Directory.systemTemp.createTemp( 'devtools-rollback', )).absolute; - print('file://${tempDir.path}'); - final tarball = io.File('${tempDir.path}/devtools.tar.gz'); - final extractDir = await io.Directory( - '${tempDir.path}/extract/', - ).absolute.create(); - final client = io.HttpClient(); - final version = argResults![_toVersionArg] as String; - print('downloading tarball to ${tarball.path}'); - final tarballRequest = await client.getUrl( - Uri.http( - 'storage.googleapis.com', - 'pub-packages/packages/devtools-$version.tar.gz', - ), - ); - final tarballResponse = await tarballRequest.close(); - await tarballResponse.pipe(tarball.openWrite()); - print('Tarball written; unzipping.'); + try { + print('file://${tempDir.path}'); + final tarball = io.File('${tempDir.path}/devtools.tar.gz'); + final extractDir = await io.Directory( + '${tempDir.path}/extract/', + ).absolute.create(); + final client = io.HttpClient(); + final version = argResults![_toVersionArg] as String; + print('downloading tarball to ${tarball.path}'); + final tarballRequest = await client.getUrl( + Uri.http( + 'storage.googleapis.com', + 'pub-packages/packages/devtools-$version.tar.gz', + ), + ); + final tarballResponse = await tarballRequest.close(); + await tarballResponse.pipe(tarball.openWrite()); + print('Tarball written; unzipping.'); - await io.Process.run('tar', [ - '-x', - '-z', - '-f', - tarball.path.split('/').last, - '-C', - extractDir.path, - ], workingDirectory: tempDir.path); - print('file://${tempDir.path}'); + await io.Process.run('tar', [ + '-x', + '-z', + '-f', + tarball.path.split('/').last, + '-C', + extractDir.path, + ], workingDirectory: tempDir.path); + print('file://${tempDir.path}'); - final buildDir = io.Directory('${repo.repoPath}/packages/devtools/build/'); - await buildDir.delete(recursive: true); - await io.Directory( - '${extractDir.path}build/', - ).rename('${repo.repoPath}/packages/devtools/build/'); + final buildDir = io.Directory( + '${repo.repoPath}/packages/devtools/build/', + ); + await buildDir.delete(recursive: true); + await io.Directory( + '${extractDir.path}build/', + ).rename('${repo.repoPath}/packages/devtools/build/'); - print( - 'Build outputs from Devtools version $version checked out and moved ' - 'to ${buildDir.path}', - ); - print( - 'To complete the rollback, go to ${repo.repoPath}/packages/devtools, ' - 'rev pubspec.yaml, update the changelog, unhide build/ from the ' - 'packages/devtools/.gitignore file, then run pub publish.', - ); - // TODO(djshuckerow): automatically rev pubspec.yaml and update the - // changelog so that the user can just run pub publish from - // packages/devtools. + print( + 'Build outputs from Devtools version $version checked out and moved ' + 'to ${buildDir.path}', + ); + print( + 'To complete the rollback, go to ${repo.repoPath}/packages/devtools, ' + 'rev pubspec.yaml, update the changelog, unhide build/ from the ' + 'packages/devtools/.gitignore file, then run pub publish.', + ); + // TODO(djshuckerow): automatically rev pubspec.yaml and update the + // changelog so that the user can just run pub publish from + // packages/devtools. + } finally { + if (await tempDir.exists()) { + await tempDir.delete(recursive: true); + } + } } } diff --git a/tool/test/license_utils_test.dart b/tool/test/license_utils_test.dart index 269d00ccd92..2a4ee5564f1 100644 --- a/tool/test/license_utils_test.dart +++ b/tool/test/license_utils_test.dart @@ -60,7 +60,7 @@ void main() { await _setupTestConfigFile(); }); - tearDownAll(() async { + tearDown(() async { await deleteDirectoryWithRetry(testDirectory); }); @@ -171,7 +171,7 @@ text that should be removed from the file. */ await _setupTestDirectoryStructure(); }); - tearDownAll(() async { + tearDown(() async { await deleteDirectoryWithRetry(testDirectory); }); From 626a51547a53d6710dfab3b7a965d883784fa0c9 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 29 Sep 2026 16:47:15 -0700 Subject: [PATCH 2/5] Update packages/devtools_app/test/test_infra/flutter_test_environment.dart Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../devtools_app/test/test_infra/flutter_test_environment.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/test/test_infra/flutter_test_environment.dart b/packages/devtools_app/test/test_infra/flutter_test_environment.dart index 506775bbd80..b2741f3ede8 100644 --- a/packages/devtools_app/test/test_infra/flutter_test_environment.dart +++ b/packages/devtools_app/test/test_infra/flutter_test_environment.dart @@ -204,7 +204,7 @@ class FlutterTestEnvironment { void finalTeardown() { if (_tempTestAppDirectory case final tempTestAppDirectory?) { final tempDirectory = Directory(tempTestAppDirectory); - if (Directory(tempTestAppDirectory).existsSync()) { + if (tempDirectory.existsSync()) { tempDirectory.deleteSync(recursive: true); } } From b814454922189fbeb61f2b757cb041d4dfaa3a83 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 29 Sep 2026 18:33:33 -0700 Subject: [PATCH 3/5] windows --- packages/devtools_shared/test/server/general_api_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/devtools_shared/test/server/general_api_test.dart b/packages/devtools_shared/test/server/general_api_test.dart index 1c57223d7ae..0a9fad7f616 100644 --- a/packages/devtools_shared/test/server/general_api_test.dart +++ b/packages/devtools_shared/test/server/general_api_test.dart @@ -186,7 +186,7 @@ void main() { expect(response.success, true); expect(response.message, isNull); expect(response.uri, isNotNull); - expect(response.uri!.toString(), endsWith(app!.directory.path)); + expect(response.uri!.toFilePath(), endsWith(app!.directory.path)); }); test('succeeds for a disconnect event when cache is empty', () async { @@ -223,7 +223,7 @@ void main() { expect(response.success, true); expect(response.message, isNull); expect(response.uri, isNotNull); - expect(response.uri!.toString(), endsWith(app!.directory.path)); + expect(response.uri!.toFilePath(), endsWith(app!.directory.path)); final disconnectResponse = await server.VmServiceHandler.detectRootPackageForVmService( @@ -236,7 +236,7 @@ void main() { expect(disconnectResponse.message, isNull); expect(disconnectResponse.uri, isNotNull); expect( - disconnectResponse.uri!.toString(), + disconnectResponse.uri!.toFilePath(), endsWith(app!.directory.path), ); }, From 2a562c602c872aa980faa05f09c02309a30d60d2 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 29 Sep 2026 18:55:37 -0700 Subject: [PATCH 4/5] dcm --- packages/devtools_shared/test/helpers/helpers.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devtools_shared/test/helpers/helpers.dart b/packages/devtools_shared/test/helpers/helpers.dart index 1235f957474..0e018996e4d 100644 --- a/packages/devtools_shared/test/helpers/helpers.dart +++ b/packages/devtools_shared/test/helpers/helpers.dart @@ -142,7 +142,7 @@ void main() async { /// /// Deletes will be retried if they fail for a period to avoid failing due to /// Windows being slow to unlock files after processes terminate. -Future deleteDirectoryWithRetry(Directory directory) async { +Future deleteDirectoryWithRetry(Directory directory) { // On Windows, trying to delete a directory immediately after the // test completes may fail with a file locking error. To avoid this, retry // the delete a few times before failing. @@ -150,7 +150,7 @@ Future deleteDirectoryWithRetry(Directory directory) async { // On DanTup's Windows PC, it can take ~5s for the delete to work sometimes // and this will probably be slower on bots. Allow a reasonable time because // taking 10s to delete is better than failing the tests for a non-bug. - await runWithRetry( + return runWithRetry( callback: () => directory.deleteSync(recursive: true), maxRetries: 20, retryDelay: const Duration(milliseconds: 500), From a853d846e2735b2e79e9ee01106415a7ba1ca6e9 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 29 Sep 2026 20:19:18 -0700 Subject: [PATCH 5/5] dcm --- packages/devtools_shared/test/helpers/helpers.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/devtools_shared/test/helpers/helpers.dart b/packages/devtools_shared/test/helpers/helpers.dart index 0e018996e4d..0f9dbeee66c 100644 --- a/packages/devtools_shared/test/helpers/helpers.dart +++ b/packages/devtools_shared/test/helpers/helpers.dart @@ -76,7 +76,7 @@ class TestDartApp { StreamSubscription? _stderrSub; Future start() async { - await _initTestApp(); + _initTestApp(); process = await Process.start(Platform.resolvedExecutable, [ '--observe=0', 'bin/main.dart', @@ -120,7 +120,7 @@ class TestDartApp { await deleteDirectoryWithRetry(directory); } - Future _initTestApp() async { + void _initTestApp() { Directory( path.join(directory.path, '.dart_tool'), ).createSync(recursive: true); @@ -142,7 +142,7 @@ void main() async { /// /// Deletes will be retried if they fail for a period to avoid failing due to /// Windows being slow to unlock files after processes terminate. -Future deleteDirectoryWithRetry(Directory directory) { +Future deleteDirectoryWithRetry(Directory directory) async { // On Windows, trying to delete a directory immediately after the // test completes may fail with a file locking error. To avoid this, retry // the delete a few times before failing. @@ -150,7 +150,7 @@ Future deleteDirectoryWithRetry(Directory directory) { // On DanTup's Windows PC, it can take ~5s for the delete to work sometimes // and this will probably be slower on bots. Allow a reasonable time because // taking 10s to delete is better than failing the tests for a non-bug. - return runWithRetry( + await runWithRetry( callback: () => directory.deleteSync(recursive: true), maxRetries: 20, retryDelay: const Duration(milliseconds: 500),