test(cli): Re-enable tests on Windows (#249)

dnys1 · web-flow · commit b130f14cc27d · 2025-03-14T15:04:24.000-07:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -56,13 +56,17 @@ final class SdkNotFound extends SdkFinderResult<Never> {
   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<SdkType> {
           )!; // 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 = <String>[];
diff --git 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
@@ -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<Directive>().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
@@ -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
@@ -20,7 +20,12 @@ Future<List<FirebaseProject>> _resolveProjectTest({
   String? globalProjectPointer,
   Map<String, Object?>? 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<List<FirebaseProject>> _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
@@ -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
@@ -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
@@ -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<void> 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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -121,6 +121,7 @@ final class CelestProject {`
`121`	`121`	`parentProject: parentProject,`
`122`	`122`	`cacheDb: cacheDb,`
`123`	`123`	`byteStore: byteStore,`
	`124`	`+ projectDb: projectDb,`
`124`	`125`	`);`
`125`	`126`	`return project;`
`126`	`127`	`}`