From 2aa05f9b0e9b83db4cd2f48f41a734dd4cfd7e68 Mon Sep 17 00:00:00 2001 From: Dillon Nys Date: Fri, 14 Mar 2025 12:37:17 -0700 Subject: [PATCH] test(cli): Re-enable tests on Windows --- .github/workflows/celest_cli.yaml | 19 ++--- apps/cli/lib/codegen/allocator.dart | 6 +- apps/cli/lib/project/celest_project.dart | 1 + apps/cli/lib/pub/pub_cache.dart | 4 +- apps/cli/lib/src/context.dart | 2 +- apps/cli/lib/src/sdk/sdk_finder.dart | 10 ++- .../test/analyzer/celest_analyzer_test.dart | 8 +- apps/cli/test/codegen/allocator_test.dart | 31 +------ apps/cli/test/commands/uninstall_test.dart | 6 +- .../firebase_config_value_solver_test.dart | 9 ++- apps/cli/test/pub/pub_cache_test.dart | 22 +++-- pubspec.yaml | 10 +-- tool/fix_pub_cache.dart | 81 +++++++++++++++++++ tool/pubspec.yaml | 10 +++ 14 files changed, 148 insertions(+), 71 deletions(-) create mode 100644 tool/fix_pub_cache.dart create mode 100644 tool/pubspec.yaml diff --git a/.github/workflows/celest_cli.yaml b/.github/workflows/celest_cli.yaml index 7b0903969..247a94736 100644 --- a/.github/workflows/celest_cli.yaml +++ b/.github/workflows/celest_cli.yaml @@ -23,29 +23,20 @@ jobs: matrix: os: - ubuntu-latest - - macos-14 - # TODO: Need to fix tests on Windows - # - windows-latest + - macos-latest + - windows-latest runs-on: ${{ matrix.os }} timeout-minutes: 20 steps: - name: Git Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2 - with: - submodules: recursive - # Needed for flutter-version-file: https://github.com/subosito/flutter-action?tab=readme-ov-file#use-version-from-pubspecyaml - - name: Install yq - if: runner.os == 'Windows' - shell: bash - run: | - curl -sL "https://github.com/mikefarah/yq/releases/download/v4.43.1/yq_windows_amd64.exe" -o yq.exe - echo "$PWD" >> "$GITHUB_PATH" - name: Setup Flutter uses: subosito/flutter-action@f2c4f6686ca8e8d6e6d0f28410eeef506ed66aff # 2.18.0 with: cache: true - # Locks version to repo constraint (for golden test alignment) - flutter-version-file: pubspec.yaml + # Because many golden tests encode the precise Dart/Flutter SDKs used to generate + # them, these values must be consistently used when running tests locally and in CI. + flutter-version: 3.29.1 - name: Setup Libsecret if: runner.os == 'Linux' run: tool/setup-ci.sh diff --git a/apps/cli/lib/codegen/allocator.dart b/apps/cli/lib/codegen/allocator.dart index c0009fd5e..0f5200d8b 100644 --- a/apps/cli/lib/codegen/allocator.dart +++ b/apps/cli/lib/codegen/allocator.dart @@ -1,3 +1,4 @@ +import 'package:celest_cli/project/project_paths.dart'; import 'package:celest_cli/src/context.dart'; import 'package:celest_cli/src/utils/error.dart'; import 'package:celest_cli/src/utils/path.dart'; @@ -37,9 +38,11 @@ final class CelestAllocator implements Allocator { required this.pathStrategy, @visibleForTesting String? packageName, @visibleForTesting String? clientPackageName, + @visibleForTesting ProjectPaths? projectPaths, }) : packageName = packageName ?? celestProject.pubspec.name, clientPackageName = - clientPackageName ?? celestProject.clientPubspec.name; + clientPackageName ?? celestProject.clientPubspec.name, + projectPaths = projectPaths ?? celestProject.projectPaths; static const _doNotPrefix = ['dart:core', 'package:meta/meta.dart']; static final Logger _logger = Logger('CelestAllocator'); @@ -50,6 +53,7 @@ final class CelestAllocator implements Allocator { final String packageName; final String clientPackageName; + final ProjectPaths projectPaths; late final _fileContext = path.Context( current: p.dirname(forFile), diff --git a/apps/cli/lib/project/celest_project.dart b/apps/cli/lib/project/celest_project.dart index e61bea65f..cb9d593a9 100644 --- a/apps/cli/lib/project/celest_project.dart +++ b/apps/cli/lib/project/celest_project.dart @@ -121,6 +121,7 @@ final class CelestProject { parentProject: parentProject, cacheDb: cacheDb, byteStore: byteStore, + projectDb: projectDb, ); return project; } diff --git a/apps/cli/lib/pub/pub_cache.dart b/apps/cli/lib/pub/pub_cache.dart index 71132e26f..9c9a9d40a 100644 --- a/apps/cli/lib/pub/pub_cache.dart +++ b/apps/cli/lib/pub/pub_cache.dart @@ -26,7 +26,7 @@ final class PubCache { 'celest_auth': '>=$currentMinorVersion <2.0.0', 'celest': '>=$currentMinorVersion <2.0.0', 'celest_core': '>=$currentMinorVersion <2.0.0', - 'objective_c': '>=2.0.0', + 'objective_c': '>=2.0.0 <8.0.0', }; static final _logger = Logger('PubCache'); @@ -101,7 +101,7 @@ final class PubCache { for (final package in packagesToFix.entries) { // Run serially to avoid flutter lock final result = await processManager - .start([ + .start(runInShell: true, [ Sdk.current.sdkType.name, 'pub', 'cache', diff --git a/apps/cli/lib/src/context.dart b/apps/cli/lib/src/context.dart index c2e7c48d6..d0a6e8f87 100644 --- a/apps/cli/lib/src/context.dart +++ b/apps/cli/lib/src/context.dart @@ -186,7 +186,7 @@ FileSystem fileSystem = const LocalFileSystem(); FileSystem get localFileSystem => const LocalFileSystem(); /// Global path context. -path.Context get p => fileSystem.path; +path.Context get p => localFileSystem.path; /// Global platform. /// diff --git a/apps/cli/lib/src/sdk/sdk_finder.dart b/apps/cli/lib/src/sdk/sdk_finder.dart index 06d7c53bb..afdcaa36d 100644 --- a/apps/cli/lib/src/sdk/sdk_finder.dart +++ b/apps/cli/lib/src/sdk/sdk_finder.dart @@ -56,13 +56,17 @@ final class SdkNotFound extends SdkFinderResult { String toString() { final buffer = StringBuffer('Could not find SDK'); if (candidates.isNotEmpty) { - buffer.writeln(' candidates:'); + buffer + ..writeln() + ..writeln(' candidates:'); for (final candidate in candidates) { buffer.writeln(' - $candidate'); } } if (searchPath.isNotEmpty) { - buffer.writeln(' search path:'); + buffer + ..writeln() + ..writeln(' search path:'); for (final path in searchPath) { buffer.writeln(' - $path'); } @@ -118,7 +122,7 @@ final class DartSdkFinder implements SdkFinder { )!; // never null when `throwOnFailure: true` } on ProcessPackageExecutableNotFoundException catch (e) { _logger.finest('Could not find Flutter SDK in PATH.', e); - return SdkNotFound(searchPath: e.candidates); + return SdkNotFound(searchPath: e.searchPath, candidates: e.candidates); } final searchPath = []; diff --git a/apps/cli/test/analyzer/celest_analyzer_test.dart b/apps/cli/test/analyzer/celest_analyzer_test.dart index d276d20b5..9ac3428bc 100644 --- a/apps/cli/test/analyzer/celest_analyzer_test.dart +++ b/apps/cli/test/analyzer/celest_analyzer_test.dart @@ -4,6 +4,8 @@ import 'package:celest/http.dart'; import 'package:celest_ast/celest_ast.dart'; import 'package:celest_cli/analyzer/analysis_result.dart'; import 'package:celest_cli/analyzer/celest_analyzer.dart'; +import 'package:celest_cli/database/cache/cache_database.dart'; +import 'package:celest_cli/database/project/project_database.dart'; import 'package:celest_cli/project/celest_project.dart'; import 'package:celest_cli/pub/pub_cache.dart'; import 'package:celest_cli/pub/pub_environment.dart'; @@ -133,7 +135,11 @@ $contents ]); await project.create(parentDirectory); final projectRoot = p.join(parentDirectory ?? d.sandbox, name); - await init(projectRoot: projectRoot); + await init( + projectRoot: projectRoot, + cacheDb: await CacheDatabase.memory(), + projectDb: ProjectDatabase.memory(), + ); return celestProject; } diff --git a/apps/cli/test/codegen/allocator_test.dart b/apps/cli/test/codegen/allocator_test.dart index 9070c5784..61f7a5e9b 100644 --- a/apps/cli/test/codegen/allocator_test.dart +++ b/apps/cli/test/codegen/allocator_test.dart @@ -1,38 +1,10 @@ import 'package:celest_cli/codegen/allocator.dart'; +import 'package:celest_cli/project/project_paths.dart'; import 'package:code_builder/code_builder.dart'; import 'package:test/test.dart'; void main() { group('CelestAllocator', () { - group('Windows', testOn: 'windows', () { - // When the temp dir (generated outputs) is on a separate drive from the source project, - // ensure that paths are absolute and not relative since there is no way to express a - // relative path between drives. - test('separate drives', () { - final reference = refer( - 'sayHello', - r'D:\workspace\celest_example\celest\functions\greeting.dart', - ); - const generatingForPath = - r'C:\Users\celest\AppData\Local\Temp\af0ceaa9\functions\greeting\sayHello.dart'; - final allocator = CelestAllocator( - forFile: generatingForPath, - pathStrategy: PathStrategy.robust, - prefixingStrategy: PrefixingStrategy.pretty, - ); - final symbol = allocator.allocate(reference); - expect(symbol, r'_$greeting.sayHello'); - expect( - allocator.imports.single, - isA().having( - (d) => d.url, - 'url', - 'file:///D:/workspace/celest_example/celest/functions/greeting.dart', - ), - ); - }); - }); - test('de-dups import prefixes', () async { // Only applies in pretty mode. const prefixingStrategy = PrefixingStrategy.pretty; @@ -50,6 +22,7 @@ void main() { prefixingStrategy: prefixingStrategy, packageName: 'celest_backend', clientPackageName: 'test_client', + projectPaths: ProjectPaths('/'), ); for (final uri in uris) { allocator.allocate(refer('A', uri.toString())); diff --git a/apps/cli/test/commands/uninstall_test.dart b/apps/cli/test/commands/uninstall_test.dart index 99b49857a..f2c2a4269 100644 --- a/apps/cli/test/commands/uninstall_test.dart +++ b/apps/cli/test/commands/uninstall_test.dart @@ -41,7 +41,7 @@ void main() { }); group('uninstall AOT', () { - test('windows', () async { + test('windows', testOn: 'windows', () async { ctx.fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows); const appData = r'C:\Users\test\AppData\Local\Microsoft\WindowsApps'; @@ -114,7 +114,7 @@ void main() { ).called(1); }); - test('macos', () async { + test('macos', testOn: 'posix', () async { ctx.fileSystem = MemoryFileSystem.test(style: FileSystemStyle.posix); final configDir = ctx.fileSystem.systemTempDirectory .childDirectory('Library') @@ -182,7 +182,7 @@ void main() { ).called(1); }); - group('linux', () { + group('linux', testOn: 'posix', () { test('deb installation', () async { ctx.fileSystem = MemoryFileSystem.test(style: FileSystemStyle.posix); final configDir = ctx.fileSystem.systemTempDirectory diff --git a/apps/cli/test/env/firebase_config_value_solver_test.dart b/apps/cli/test/env/firebase_config_value_solver_test.dart index 1ce7b954e..67b751f9e 100644 --- a/apps/cli/test/env/firebase_config_value_solver_test.dart +++ b/apps/cli/test/env/firebase_config_value_solver_test.dart @@ -20,7 +20,12 @@ Future> _resolveProjectTest({ String? globalProjectPointer, Map? firebaseJson, }) async { - fileSystem = MemoryFileSystem.test(); + fileSystem = MemoryFileSystem.test( + style: switch (platform.operatingSystem) { + 'windows' => FileSystemStyle.windows, + _ => FileSystemStyle.posix, + }, + ); final appDir = fileSystem.systemTempDirectory.createTempSync('app_'); if (firebaseJson != null) { appDir @@ -34,7 +39,7 @@ Future> _resolveProjectTest({ } if (globalProjectPointer != null) { final configPath = fileSystem.path.join( - platform.environment['HOME']!, + (platform.environment['HOME'] ?? platform.environment['USERPROFILE'])!, '.config', 'configstore', 'firebase-tools.json', diff --git a/apps/cli/test/pub/pub_cache_test.dart b/apps/cli/test/pub/pub_cache_test.dart index b77d32d7d..7b19fe11d 100644 --- a/apps/cli/test/pub/pub_cache_test.dart +++ b/apps/cli/test/pub/pub_cache_test.dart @@ -1,4 +1,5 @@ import 'package:celest_cli/pub/pub_cache.dart'; +import 'package:celest_cli/src/context.dart'; import 'package:test/test.dart'; import '../common.dart'; @@ -24,11 +25,20 @@ void main() { } }); - test('fixes cache', timeout: Timeout.none, () async { - final result = await pubCache.repair(); - expect(result.exitCode, 0, reason: '${result.stdout}\n${result.stderr}'); - final numFixed = await pubCache.fix(throwOnError: true); - expect(numFixed, greaterThan(PubCache.packagesToFix.length)); - }); + test( + 'fixes cache', + timeout: Timeout.none, + skip: !platform.environment.containsKey('CI'), + () async { + final result = await pubCache.repair(); + expect( + result.exitCode, + 0, + reason: '${result.stdout}\n${result.stderr}', + ); + final numFixed = await pubCache.fix(throwOnError: true); + expect(numFixed, greaterThan(PubCache.packagesToFix.length)); + }, + ); }); } diff --git a/pubspec.yaml b/pubspec.yaml index d7ef6f5e1..468f8a099 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,16 +1,8 @@ name: celest_dev publish_to: none -# The version locks for GitHub actions. -# -# Because many golden tests encode the precise Dart/Flutter SDKs used to generate -# them, these values must be consistently used when running tests locally and in CI. -# -# However, setting these locks in the actual packages is too restrictive when -# switching between versions, so they are instead centrally managed here. environment: - sdk: 3.7.0 - flutter: 3.29.1 + sdk: ^3.7.0 dev_dependencies: melos: ^5.3.0 diff --git a/tool/fix_pub_cache.dart b/tool/fix_pub_cache.dart new file mode 100644 index 000000000..d6dd53b87 --- /dev/null +++ b/tool/fix_pub_cache.dart @@ -0,0 +1,81 @@ +import 'dart:io'; + +import 'package:path/path.dart' as p; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:yaml_edit/yaml_edit.dart'; + +const fixPackages = [ + 'native_storage', + 'native_authentication', + 'jni', + 'objective_c', +]; + +Future main() async { + final pubCache = p.join( + Platform.environment['PUB_CACHE'] ?? + p.join(Platform.environment['HOME']!, '.pub-cache'), + 'hosted', + 'pub.dev', + ); + final pubCacheDir = Directory(pubCache); + if (!pubCacheDir.existsSync()) { + print('No pub cache found at $pubCache'); + exit(1); + } + + for (final package in fixPackages) { + print('Adding $package to pub cache'); + final res = await Process.run(Platform.resolvedExecutable, [ + 'pub', + 'cache', + 'add', + package, + '--all', + ]); + if (res.exitCode != 0) { + print('Failed to add $package to pub cache: ${res.stderr}'); + exit(1); + } + } + + await for (final packageDir in pubCacheDir.list()) { + print('Checking ${packageDir.path}'); + if (packageDir is! Directory) { + continue; + } + var fixPackage = false; + for (final packageToFix in fixPackages) { + if (p.basename(packageDir.path).startsWith('$packageToFix-')) { + fixPackage = true; + break; + } + } + if (!fixPackage) { + continue; + } + final pubspecFile = File(p.join(packageDir.path, 'pubspec.yaml')); + if (!pubspecFile.existsSync()) { + continue; + } + final pubspecYaml = await pubspecFile.readAsString(); + final pubspec = Pubspec.parse(pubspecYaml); + if (!fixPackages.contains(pubspec.name)) { + continue; + } + final needsEnvFix = pubspec.environment.containsKey('flutter'); + final needsDepsFix = pubspec.dependencies.containsKey('flutter'); + if (!needsEnvFix && !needsDepsFix) { + continue; + } + final editor = YamlEditor(pubspecYaml); + if (needsEnvFix) { + editor.remove(['environment', 'flutter']); + } + if (needsDepsFix) { + editor.remove(['dependencies', 'flutter']); + } + await pubspecFile.writeAsString(editor.toString()); + print('Fixed pubspec for ${pubspec.name} in ${packageDir.path}'); + } +} diff --git a/tool/pubspec.yaml b/tool/pubspec.yaml new file mode 100644 index 000000000..be29557b7 --- /dev/null +++ b/tool/pubspec.yaml @@ -0,0 +1,10 @@ +name: celest_tool +publish_to: none + +environment: + sdk: ^3.3.0 + +dependencies: + path: any + pubspec_parse: ^1.2.3 + yaml_edit: any