fix(shorebird_ci): path-derived job keys + has_unit_tests gate (#3763)

packages/shorebird_ci/CHANGELOG.md

@@ -1,5 +1,10 @@
 <!-- cspell:words toplevel -->
 
+# 0.2.1
+
+- Static main workflow YAML map keys default to each package's `name:` and fall back to `<parent_dir>_<package_name>` only when two or more packages share a name. Avoids prefixing in the common case while still handling duplicate-`name:` repos. The `package_name:` input passed to the reusable workflow keeps the actual package name for codecov flag display.
+- Static reusable workflows now gate the test and codecov-upload steps on a new `has_unit_tests` input. Packages without a `test/` directory no longer attempt to run `dart test` / `flutter test` or upload coverage. The input defaults to `true` for back-compat with workflows that don't pass it.
+
 # 0.2.0
 
 - Generated dynamic workflow now pins `fetch-depth: 0` on the setup-job checkout. Without it, `affected_packages` fails on every PR because the default shallow checkout doesn't include `origin/main`. The static main workflow gets the same fix so dorny/paths-filter can diff on push events.

packages/shorebird_ci/lib/src/commands/generate_command.dart

@@ -7,6 +7,7 @@ import 'package:shorebird_ci/src/commands/repo_root_option.dart';
 import 'package:shorebird_ci/src/dependency_resolver.dart';
 import 'package:shorebird_ci/src/flutter_version_resolver.dart';
 import 'package:shorebird_ci/src/package_description.dart';
+import 'package:shorebird_ci/src/package_slug.dart';
 import 'package:shorebird_ci/src/repository_analyzer.dart';
 import 'package:shorebird_ci/src/repository_description.dart';
 
@@ -209,6 +210,11 @@ updates:
     required List<PackageDescription> packages,
   }) {
     final resolver = DependencyResolver(repository.root.path);
+    final slugs = computePackageSlugs(
+      packages: packages,
+      repoRoot: repository.root.path,
+    );
+
     final buffer = StringBuffer()
       ..write('''
 # Generated by shorebird_ci --style static. Safe to edit.
@@ -230,10 +236,8 @@ jobs:
 ''');
 
     for (final package in packages) {
-      buffer.writeln(
-        '      ${package.name}: '
-        '\${{ steps.filter.outputs.${package.name} }}',
-      );
+      final slug = slugs[package]!;
+      buffer.writeln('      $slug: \${{ steps.filter.outputs.$slug }}');
     }
 
     // Verify first so we fail fast if the dorny filters below have
@@ -261,8 +265,9 @@ jobs:
         from: repository.root.path,
       );
       final sortedDeps = resolver.resolve(packageDir).toList()..sort();
+      final slug = slugs[package]!;
 
-      buffer.writeln('            ${package.name}:');
+      buffer.writeln('            $slug:');
       for (final dep in sortedDeps) {
         buffer.writeln('              - $dep/**');
       }
@@ -285,16 +290,18 @@ jobs:
       final reusable = isFlutter
           ? '_shorebird_ci_flutter.yaml'
           : '_shorebird_ci_dart.yaml';
+      final slug = slugs[package]!;
 
       buffer.write('''
-  ${package.name}:
+  $slug:
     needs: changes
-    if: needs.changes.outputs.${package.name} == 'true'
+    if: needs.changes.outputs.$slug == 'true'
     uses: ./.github/workflows/$reusable
     with:
       package_name: ${package.name}
       package_path: $packageDir
       has_bloc_lint: ${RepositoryAnalyzer.dependsOnBlocLint(root: package.root)}
+      has_unit_tests: ${RepositoryAnalyzer.hasUnitTests(root: package.root)}
       subpackages: "${subpackages.join(' ')}"
 ''');
 
@@ -324,18 +331,21 @@ jobs:
     final testStep = hasCodecov
         ? r'''
       - name: Run Tests
+        if: inputs.has_unit_tests
         working-directory: ${{ inputs.package_path }}
         run: |
           dart pub global activate coverage && \
           dart test --coverage=coverage && \
           dart pub global run coverage:format_coverage --lcov --in=coverage --out=coverage/lcov.info --report-on=lib --check-ignore
-      - uses: codecov/codecov-action@v5
+      - if: inputs.has_unit_tests
+        uses: codecov/codecov-action@v5
         with:
           flags: ${{ inputs.package_name }}
           working-directory: ${{ inputs.package_path }}
 '''
         : r'''
-      - working-directory: ${{ inputs.package_path }}
+      - if: inputs.has_unit_tests
+        working-directory: ${{ inputs.package_path }}
         run: dart test
 ''';
 
@@ -355,6 +365,10 @@ on:
         required: false
         default: false
         type: boolean
+      has_unit_tests:
+        required: false
+        default: true
+        type: boolean
       subpackages:
         required: false
         default: ""
@@ -392,15 +406,18 @@ $testStep''';
   String _buildFlutterReusableWorkflow({required bool hasCodecov}) {
     final testStep = hasCodecov
         ? r'''
-      - working-directory: ${{ inputs.package_path }}
+      - if: inputs.has_unit_tests
+        working-directory: ${{ inputs.package_path }}
         run: flutter test --coverage
-      - uses: codecov/codecov-action@v5
+      - if: inputs.has_unit_tests
+        uses: codecov/codecov-action@v5
         with:
           flags: ${{ inputs.package_name }}
           working-directory: ${{ inputs.package_path }}
 '''
         : r'''
-      - working-directory: ${{ inputs.package_path }}
+      - if: inputs.has_unit_tests
+        working-directory: ${{ inputs.package_path }}
         run: flutter test
 ''';
 
@@ -428,6 +445,10 @@ on:
         required: false
         default: false
         type: boolean
+      has_unit_tests:
+        required: false
+        default: true
+        type: boolean
       subpackages:
         required: false
         default: ""

packages/shorebird_ci/lib/src/commands/verify_command.dart

@@ -6,6 +6,7 @@ import 'package:shorebird_ci/src/commands/repo_root_option.dart';
 import 'package:shorebird_ci/src/dependency_resolver.dart';
 import 'package:shorebird_ci/src/dorny_filter.dart';
 import 'package:shorebird_ci/src/package_description.dart';
+import 'package:shorebird_ci/src/package_slug.dart';
 import 'package:shorebird_ci/src/repository_analyzer.dart';
 
 /// Marker comment that the `generate` command writes into dynamic
@@ -21,9 +22,11 @@ const dynamicCoverageMarker = '# shorebird_ci-managed: dynamic';
 ///   - **Dynamic**: a workflow that calls `shorebird_ci affected_packages`
 ///     covers every package automatically. One dynamic workflow means no
 ///     missing packages.
-///   - **Static**: each package name appears in a `dorny/paths-filter`
-///     block somewhere. Missing packages are reported with the dorny
-///     entry that should be added (including transitive deps).
+///   - **Static**: each package's slug (see [computePackageSlugs]) appears
+///     in a `dorny/paths-filter` block somewhere. For most packages the
+///     slug is just the package name; when two packages share a name the
+///     slug is `<parent_dir>_<name>`. Missing packages are reported with
+///     the dorny entry that should be added (including transitive deps).
 class VerifyCommand extends Command<int> with RepoRootOption {
   /// Creates a [VerifyCommand].
   VerifyCommand() {
@@ -98,15 +101,19 @@ class VerifyCommand extends Command<int> with RepoRootOption {
       return 0;
     }
 
+    final slugs = computePackageSlugs(
+      packages: allPackages,
+      repoRoot: repoRoot,
+    );
     final missing = <PackageDescription>[];
 
     for (final pkg in allPackages) {
       if (ignoreSet.contains(pkg.name)) continue;
-      final workflows = coverageMap[pkg.name];
+      final slug = slugs[pkg]!;
+      final workflows = coverageMap[slug];
       if (workflows != null) {
-        stdout.writeln(
-          'OK: ${pkg.name} (${workflows.join(', ')})',
-        );
+        final label = slug == pkg.name ? pkg.name : '${pkg.name} ($slug)';
+        stdout.writeln('OK: $label (${workflows.join(', ')})');
       } else {
         missing.add(pkg);
       }
@@ -124,11 +131,13 @@ class VerifyCommand extends Command<int> with RepoRootOption {
       final packageDir = posixRelative(pkg.rootPath, from: repoRoot);
       final deps = resolver.resolve(packageDir);
       final sortedDeps = deps.toList()..sort();
+      final slug = slugs[pkg]!;
+      final label = slug == pkg.name ? pkg.name : '${pkg.name} ($slug)';
 
       stdout
-        ..writeln('MISSING: ${pkg.name}')
+        ..writeln('MISSING: $label')
         ..writeln('  Add this entry to a dorny paths-filter block:')
-        ..writeln('            ${pkg.name}:');
+        ..writeln('            $slug:');
       for (final dep in sortedDeps) {
         stdout.writeln('              - $dep/**');
       }

packages/shorebird_ci/lib/src/package_slug.dart

@@ -0,0 +1,50 @@
+import 'package:shorebird_ci/src/package_description.dart';
+import 'package:shorebird_ci/src/repository_analyzer.dart';
+
+/// Maps each package to the YAML map key used for its dorny filter,
+/// changes-job output, and per-package job in the static main workflow.
+///
+/// A package's slug defaults to its `name:`. When two or more packages
+/// share a `name:`, those packages fall back to `<parent_dir>_<name>`
+/// to disambiguate. Parent dirs are normalized to pub's
+/// `[a-z][a-z0-9_]*` rules so the slug can be used in GitHub Actions
+/// expressions. `generate` writes these slugs and `verify` reads them,
+/// so both commands must agree on the rule.
+Map<PackageDescription, String> computePackageSlugs({
+  required List<PackageDescription> packages,
+  required String repoRoot,
+}) {
+  final byName = <String, List<PackageDescription>>{};
+  for (final pkg in packages) {
+    byName.putIfAbsent(pkg.name, () => []).add(pkg);
+  }
+  final duplicateNames = {
+    for (final entry in byName.entries)
+      if (entry.value.length > 1) entry.key,
+  };
+
+  return {
+    for (final pkg in packages)
+      pkg: _slugFor(
+        package: pkg,
+        repoRoot: repoRoot,
+        duplicateNames: duplicateNames,
+      ),
+  };
+}
+
+String _slugFor({
+  required PackageDescription package,
+  required String repoRoot,
+  required Set<String> duplicateNames,
+}) {
+  if (!duplicateNames.contains(package.name)) return package.name;
+  final relative = posixRelative(package.rootPath, from: repoRoot);
+  final parts = relative.split('/');
+  if (parts.length < 2) return package.name;
+  final parent = parts[parts.length - 2].toLowerCase().replaceAll(
+    RegExp('[^a-z0-9_]'),
+    '_',
+  );
+  return '${parent}_${package.name}';
+}

packages/shorebird_ci/lib/src/repository_analyzer.dart

@@ -87,8 +87,6 @@ class RepositoryAnalyzer {
         .whereType<PackageDescription>()
         .toList();
 
-    _checkForDuplicateNames(packageDescriptions);
-
     return RepositoryDescription(
       packages: packageDescriptions,
       root: repositoryRoot,
@@ -97,29 +95,6 @@ class RepositoryAnalyzer {
     );
   }
 
-  static void _checkForDuplicateNames(
-    List<PackageDescription> packages,
-  ) {
-    final byName = <String, List<PackageDescription>>{};
-    for (final pkg in packages) {
-      byName.putIfAbsent(pkg.name, () => []).add(pkg);
-    }
-    final duplicates = byName.entries.where((e) => e.value.length > 1);
-    if (duplicates.isEmpty) return;
-
-    final buffer = StringBuffer(
-      'Duplicate package names found. Each package in the workspace '
-      'must have a unique `name:` in its pubspec.yaml.\n',
-    );
-    for (final entry in duplicates) {
-      buffer.writeln('  ${entry.key}:');
-      for (final pkg in entry.value) {
-        buffer.writeln('    - ${pkg.rootPath}');
-      }
-    }
-    throw StateError(buffer.toString().trimRight());
-  }
-
   /// Allowed characters in a package or subpackage path: letters,
   /// digits, `_`, `-`, `.`, and `/`. Any other character (whitespace,
   /// shell metacharacters, Unicode) is rejected.
@@ -144,10 +119,9 @@ class RepositoryAnalyzer {
 
   /// Pub's own naming convention for packages: lowercase letter or
   /// underscore start, then lowercase letters, digits, and underscores.
-  /// `pubspec.yaml` is just YAML — pub doesn't gate this name until you
-  /// publish — so a malformed name can land here and get embedded as
-  /// a YAML map key in the generated workflow. Validate at analysis
-  /// time instead.
+  /// `pubspec.yaml` is just YAML, and pub doesn't gate this name until
+  /// publish. A malformed name flows into the slug used as a YAML map
+  /// key in the generated workflow, so validate at analysis time.
   static final _safePackageNameRegex = RegExp(r'^[a-z][a-z0-9_]*$');
 
   static void _requireSafePackageName(String name, {required String source}) {

packages/shorebird_ci/pubspec.yaml

@@ -3,7 +3,7 @@ description: >-
   CI tooling for Dart and Flutter monorepos. Generates GitHub Actions
   workflows, resolves affected packages via dependency graphs, and
   verifies path filters stay in sync.
-version: 0.2.0
+version: 0.2.1
 homepage: https://shorebird.dev
 repository: https://github.com/shorebirdtech/shorebird/tree/main/packages/shorebird_ci
 topics: [ci, github-actions, monorepo, shorebird]

packages/shorebird_ci/test/generate_command_test.dart

@@ -261,6 +261,110 @@ void main() {
       },
     );
 
+    test(
+      'has_unit_tests gates the test + codecov steps',
+      () async {
+        // Two packages: one with a test/ dir, one without. The main
+        // workflow should pass has_unit_tests per package, and the
+        // reusable workflow should gate its test step on the input.
+        createPackage(
+          tempDir,
+          'packages/with_tests',
+          'with_tests',
+          addTestDir: true,
+        );
+        createPackage(tempDir, 'packages/no_tests', 'no_tests');
+        initGitRepo(tempDir);
+
+        await runGenerate(tempDir, extra: ['--style', 'static']);
+
+        final main = _readMain(tempDir);
+        expect(
+          main,
+          matches(
+            RegExp(
+              'with_tests:[^#]*has_unit_tests: true',
+              dotAll: true,
+            ),
+          ),
+        );
+        expect(
+          main,
+          matches(
+            RegExp(
+              'no_tests:[^#]*has_unit_tests: false',
+              dotAll: true,
+            ),
+          ),
+        );
+
+        final reusable = File(
+          p.join(
+            tempDir.path,
+            '.github',
+            'workflows',
+            '_shorebird_ci_dart.yaml',
+          ),
+        ).readAsStringSync();
+        expect(reusable, contains('has_unit_tests:'));
+        expect(reusable, contains('if: inputs.has_unit_tests'));
+      },
+    );
+
+    test(
+      'YAML map keys default to the package name when unique',
+      () async {
+        // A repo where every package has a unique `name:` — the YAML
+        // keys are just the names, no path prefix.
+        createPackage(tempDir, 'packages/foo', 'foo');
+        createPackage(tempDir, 'apps/bar', 'bar');
+        initGitRepo(tempDir);
+
+        await runGenerate(tempDir, extra: ['--style', 'static']);
+        final yaml = _readMain(tempDir);
+
+        for (final slug in ['foo', 'bar']) {
+          expect(yaml, contains('$slug:'));
+          expect(yaml, contains('needs.changes.outputs.$slug'));
+        }
+        // No path-prefixed variants leak in.
+        expect(yaml, isNot(contains('packages_foo')));
+        expect(yaml, isNot(contains('apps_bar')));
+      },
+    );
+
+    test(
+      'YAML map keys fall back to <parent>_<name> on collision',
+      () async {
+        // Two packages w/ the same `name:` at different parent dirs:
+        // pub allows this, and shorebird_ci must disambiguate the YAML
+        // keys by walking up one path segment. `name: harness` under
+        // `apps/alpha/` becomes `alpha_harness`, under `apps/beta/`
+        // becomes `beta_harness`. A unique-named neighbor in the same
+        // repo keeps its plain name.
+        createPackage(tempDir, 'apps/alpha/harness', 'harness');
+        createPackage(tempDir, 'apps/beta/harness', 'harness');
+        createPackage(tempDir, 'packages/foo', 'foo');
+        initGitRepo(tempDir);
+
+        await runGenerate(tempDir, extra: ['--style', 'static']);
+        final yaml = _readMain(tempDir);
+
+        for (final slug in ['alpha_harness', 'beta_harness', 'foo']) {
+          // Each slug appears as a dorny filter key, an outputs entry,
+          // and a per-package job key.
+          expect(yaml, contains('$slug:'));
+          expect(yaml, contains('needs.changes.outputs.$slug'));
+        }
+        // The actual package name still flows through as the
+        // `package_name:` input value (used for codecov flags).
+        expect(yaml, contains('package_name: harness'));
+        expect(yaml, contains('package_name: foo'));
+        // The unique package does not get prefixed.
+        expect(yaml, isNot(contains('packages_foo')));
+      },
+    );
+
     test('verify step comes before dorny filter', () async {
       createPackage(tempDir, 'packages/foo', 'foo');
       initGitRepo(tempDir);

packages/shorebird_ci/test/repository_analyzer_test.dart

@@ -150,25 +150,21 @@ void main() {
       );
     });
 
-    test('throws on duplicate package names', () async {
-      // Two packages declaring `name: example` would silently collide
-      // when used as YAML map keys in the generated workflow. Fail
-      // loudly instead.
+    test('allows packages sharing a `name:` at different paths', () async {
+      // Two packages can legitimately share a `name:` (pub allows it
+      // when the packages are unrelated, e.g. test harnesses sitting
+      // next to multiple apps). The analyzer must surface both; the
+      // workflow generator disambiguates by path when it emits YAML
+      // map keys.
       createPackage(tempDir, 'a/example', 'example');
       createPackage(tempDir, 'b/example', 'example');
       initGitRepo(tempDir);
 
       final analyzer = RepositoryAnalyzer();
-      expect(
-        () => analyzer.analyze(repositoryRoot: tempDir),
-        throwsA(
-          isA<StateError>().having(
-            (e) => e.message,
-            'message',
-            contains('Duplicate package names'),
-          ),
-        ),
-      );
+      final repo = analyzer.analyze(repositoryRoot: tempDir);
+
+      expect(repo.packages.length, 2);
+      expect(repo.packages.every((p) => p.name == 'example'), isTrue);
     });
 
     test(

packages/shorebird_ci/test/verify_command_test.dart

@@ -125,6 +125,37 @@ jobs:
     expect(await runVerify(tempDir), 1);
   });
 
+  test(
+    'colliding package names match by slug, not by name',
+    () async {
+      // Two packages share `name: harness` at different parent dirs.
+      // `generate` emits the dorny filter keys as `alpha_harness` and
+      // `beta_harness`. Verify must compute the same slugs to
+      // recognize coverage; looking up by plain `pkg.name`
+      // ("harness") would miss both.
+      createPackage(tempDir, 'apps/alpha/harness', 'harness');
+      createPackage(tempDir, 'apps/beta/harness', 'harness');
+      _writeWorkflow(tempDir, 'ci.yaml', '''
+name: CI
+on: [push]
+jobs:
+  changes:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: dorny/paths-filter@v3
+        with:
+          filters: |
+            alpha_harness:
+              - apps/alpha/harness/**
+            beta_harness:
+              - apps/beta/harness/**
+''');
+      initGitRepo(tempDir);
+
+      expect(await runVerify(tempDir), 0);
+    },
+  );
+
   test('exposes a non-empty description', () {
     expect(VerifyCommand().description, isNotEmpty);
   });