From b9bf4f2963b25f8edb207b514c3d0a314294ee87 Mon Sep 17 00:00:00 2001 From: bin <17426470+boyan01@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:02:23 +0800 Subject: [PATCH 1/2] ci: run only affected integration targets Narrow integration target selection so shared files no longer fan out to every target: - .github/integration-tests.json: select only targets added or edited relative to the manifest at the PR merge base; removed targets need no job, and a manifest missing at the base selects every registered target. - packages//pubspec.yaml: select the member's package plus every workspace package with a member that depends on it, directly or transitively, through dependencies or dev_dependencies. - Root pubspec.yaml alone selects nothing; pubspec.lock, .github/workflows/ and tool/ci/ still select every target. The CI validate job now fails when dart pub get changes pubspec.lock, so a resolution change cannot skip targets through an uncommitted lockfile. --- .github/workflows/ci.yml | 8 ++ tool/ci/select_integration_targets.py | 80 ++++++++++-- tool/ci/test_select_integration_targets.py | 143 ++++++++++++++++++++- 3 files changed, 217 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28f95544..7f4ebd08 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,14 @@ jobs: - name: Install workspace dependencies run: dart pub get + # Integration target selection treats an unchanged lockfile as unchanged resolution. + - name: Check committed lockfile + run: | + if ! git diff --exit-code pubspec.lock; then + echo "::error file=pubspec.lock::dart pub get changed pubspec.lock. Run dart pub get and commit pubspec.lock." + exit 1 + fi + - name: Set PR diff base if: github.event_name == 'pull_request' run: | diff --git a/tool/ci/select_integration_targets.py b/tool/ci/select_integration_targets.py index 27142f2e..fa76bdcf 100644 --- a/tool/ci/select_integration_targets.py +++ b/tool/ci/select_integration_targets.py @@ -70,17 +70,78 @@ def changed_files(root, base, head): return output.decode().split('\0')[:-1] -def select_targets(targets, paths): +def read_pubspec(text): + """Read the package name, workspace members, and direct dependency names from pubspec text. + + Only the top-level keys this selector needs are parsed, line by line, because + the standard library has no YAML parser. + """ + pubspec = {'name': None, 'workspace': [], 'dependencies': set()} + section = child_indent = None + for line in text.splitlines(): + content = line.split('#', 1)[0].rstrip() + if not content.strip(): + continue + indent = len(content) - len(content.lstrip()) + if indent == 0: + section, _, value = content.partition(':') + child_indent = None + if section == 'name': + pubspec['name'] = value.strip().strip('\'"') + continue + if child_indent is None: + child_indent = indent + if section == 'workspace' and content.lstrip().startswith('- '): + pubspec['workspace'].append(content.lstrip()[2:].strip().strip('\'"').rstrip('/')) + elif section in {'dependencies', 'dev_dependencies'} and indent == child_indent: + match = re.fullmatch(r'\s*([A-Za-z_][A-Za-z0-9_]*)\s*:.*', content) + if match: + pubspec['dependencies'].add(match.group(1)) + return pubspec + + +def load_workspace(root): + """Map each workspace member directory to its parsed pubspec.""" + members = read_pubspec((root / 'pubspec.yaml').read_text())['workspace'] + return {member: read_pubspec((root / member / 'pubspec.yaml').read_text()) for member in members} + + +def base_manifest_targets(root, base, head): + """Return the registered targets at the PR merge base, or none when the manifest did not exist.""" + merge_base = subprocess.check_output(['git', 'merge-base', base, head], cwd=root).decode().strip() + revision = f'{merge_base}:.github/integration-tests.json' + if subprocess.run(['git', 'cat-file', '-e', revision], cwd=root, capture_output=True).returncode: + return [] + return json.loads(subprocess.check_output(['git', 'show', revision], cwd=root))['targets'] + + +def select_targets(targets, paths, base_targets=(), workspace=None): if paths is None: return targets - shared = any( - path in {'pubspec.yaml', 'pubspec.lock', '.github/integration-tests.json'} - or path.startswith(('.github/workflows/', 'tool/ci/')) - or (path.startswith('packages/') and path.endswith('/pubspec.yaml')) + # Lockfile and CI changes can affect every job. + if any(path == 'pubspec.lock' or path.startswith(('.github/workflows/', 'tool/ci/')) for path in paths): + return targets + workspace = workspace or {} + packages = {path.split('/')[1] for path in paths if path.startswith('packages/') and path.count('/') > 1} + # A member pubspec change also affects workspace members that depend on it, directly or transitively. + names = { + workspace[path.removesuffix('/pubspec.yaml')]['name'] for path in paths - ) - return [target for target in targets if shared or any( - path.startswith(f'packages/{target["package"]}/') for path in paths + if path.endswith('/pubspec.yaml') and path.removesuffix('/pubspec.yaml') in workspace + } + while dependents := { + pubspec['name'] for pubspec in workspace.values() if pubspec['dependencies'] & names + } - names: + names |= dependents + packages |= { + member.split('/')[1] + for member, pubspec in workspace.items() + if member.startswith('packages/') and pubspec['name'] in names + } + # A manifest change affects only added or edited targets; removed targets need no job. + manifest_changed = '.github/integration-tests.json' in paths + return [target for target in targets if target['package'] in packages or ( + manifest_changed and target not in base_targets )] @@ -92,7 +153,8 @@ def main(): args = parser.parse_args() targets = load_targets(ROOT) paths = changed_files(ROOT, args.base, args.head) if args.base else None - selected = select_targets(targets, paths) + base_targets = base_manifest_targets(ROOT, args.base, args.head) if args.base else [] + selected = select_targets(targets, paths, base_targets, load_workspace(ROOT)) matrix = json.dumps({'include': selected}, separators=(',', ':')) print(matrix) if args.output: diff --git a/tool/ci/test_select_integration_targets.py b/tool/ci/test_select_integration_targets.py index 9fe59e76..02d70d34 100644 --- a/tool/ci/test_select_integration_targets.py +++ b/tool/ci/test_select_integration_targets.py @@ -8,12 +8,21 @@ import tempfile import unittest -from select_integration_targets import ROOT, changed_files, load_targets, select_targets +from select_integration_targets import ( + ROOT, + base_manifest_targets, + changed_files, + load_targets, + load_workspace, + read_pubspec, + select_targets, +) class TargetSelectionTest(unittest.TestCase): def setUp(self): self.targets = load_targets(ROOT) + self.workspace = load_workspace(ROOT) def test_related_and_unrelated_paths(self): native = [target for target in self.targets if target['package'] == 'desktop_webview_window'] @@ -26,16 +35,103 @@ def test_related_and_unrelated_paths(self): 'packages/desktop_drop/test/browser/web_drop_test.dart': browser, 'packages/desktop_webview_window_other/lib/test.dart': [], 'README.md': [], - 'pubspec.yaml': self.targets, + 'pubspec.yaml': [], 'pubspec.lock': self.targets, - 'packages/desktop_drop/pubspec.yaml': self.targets, - '.github/integration-tests.json': self.targets, + 'packages/desktop_drop/pubspec.yaml': browser, + 'packages/desktop_drop/example/pubspec.yaml': browser, + '.github/integration-tests.json': [], '.github/workflows/integration-tests.yml': self.targets, + '.github/workflows/ci.yml': self.targets, 'tool/ci/select_integration_targets.py': self.targets, } for path, expected in cases.items(): with self.subTest(path=path): - self.assertEqual(select_targets(self.targets, [path]), expected) + self.assertEqual(select_targets(self.targets, [path], self.targets, self.workspace), expected) + + def test_read_pubspec(self): + pubspec = read_pubspec('''name: 'sample' # comment +environment: + sdk: ^3.12.0 +workspace: + - packages/first + - "packages/first/example/" # trailing slash +dependencies: + flutter: + sdk: flutter + + # commented_out: ^1.0.0 + local: + path: ../local + hosted: ^1.0.0 +dev_dependencies: + lints: ^6.0.0 + git_package: + git: + url: https://example.invalid/repo.git#main +dependency_overrides: + overridden: ^1.0.0 +flutter: + plugin: + platforms: + linux: + pluginClass: SamplePlugin +''') + self.assertEqual(pubspec, { + 'name': 'sample', + 'workspace': ['packages/first', 'packages/first/example'], + 'dependencies': {'flutter', 'local', 'hosted', 'lints', 'git_package'}, + }) + + def test_real_workspace_dependencies(self): + self.assertEqual(self.workspace['packages/desktop_drop']['name'], 'desktop_drop') + self.assertIn('desktop_drop', self.workspace['packages/desktop_drop/example']['dependencies']) + targets = [ + {'package': 'mixin_logger', 'platform': 'linux'}, + {'package': 'desktop_multi_window', 'platform': 'linux'}, + {'package': 'win_toast', 'platform': 'windows'}, + {'package': 'desktop_drop', 'platform': 'web'}, + ] + # Both examples depend on mixin_logger. + self.assertEqual( + select_targets(targets, ['packages/mixin_logger/pubspec.yaml'], workspace=self.workspace), + targets[:3], + ) + + def test_package_pubspec_selects_transitive_dependents(self): + workspace = { + 'packages/base': {'name': 'base', 'workspace': [], 'dependencies': {'flutter'}}, + 'packages/base/example': {'name': 'base_example', 'workspace': [], 'dependencies': {'base'}}, + 'packages/middle': {'name': 'middle', 'workspace': [], 'dependencies': {'base'}}, + 'packages/top': {'name': 'top', 'workspace': [], 'dependencies': {'middle'}}, + 'packages/other': {'name': 'other', 'workspace': [], 'dependencies': {'flutter'}}, + } + targets = [{'package': package, 'platform': 'linux'} for package in ['base', 'middle', 'top', 'other']] + cases = { + 'packages/base/pubspec.yaml': targets[:3], + 'packages/base/example/pubspec.yaml': targets[:1], + 'packages/middle/pubspec.yaml': targets[1:3], + 'packages/top/pubspec.yaml': targets[2:3], + 'packages/base/lib/base.dart': targets[:1], + 'packages/removed/pubspec.yaml': [], + 'pubspec.yaml': [], + } + for path, expected in cases.items(): + with self.subTest(path=path): + self.assertEqual(select_targets(targets, [path], workspace=workspace), expected) + + def test_manifest_change_selects_added_and_edited_targets(self): + unchanged = {'package': 'unchanged', 'platform': 'linux', 'runner': 'ubuntu-latest'} + edited = {'package': 'edited', 'platform': 'macos', 'runner': 'macos-latest'} + removed = {'package': 'removed', 'platform': 'linux', 'runner': 'ubuntu-latest'} + added = {'package': 'added', 'platform': 'windows', 'runner': 'windows-latest'} + base = [unchanged, edited, removed] + head = [unchanged, {**edited, 'runner': 'macos-15'}, added] + manifest = '.github/integration-tests.json' + self.assertEqual(select_targets(head, [manifest], base), head[1:]) + self.assertEqual(select_targets(head, [manifest], head), []) + self.assertEqual(select_targets(head, [manifest], []), head) + # Unrelated targets in the base manifest do not matter when it is unchanged. + self.assertEqual(select_targets(head, ['packages/unchanged/lib/a.dart'], []), head[:1]) def test_all_and_empty(self): self.assertEqual(select_targets(self.targets, None), self.targets) @@ -118,6 +214,43 @@ def commit(path): with self.assertRaises(subprocess.CalledProcessError): changed_files(root, 'missing-base', 'HEAD') + def test_base_manifest_uses_merge_base_and_handles_missing_manifest(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + + def git(*args): + return subprocess.check_output(['git', *args], cwd=root, stderr=subprocess.DEVNULL).decode().strip() + + def commit(path, content): + file = root / path + file.parent.mkdir(parents=True, exist_ok=True) + file.write_text(content) + git('add', '.') + git('commit', '-m', 'test fixture') + return git('rev-parse', 'HEAD') + + def manifest(*packages): + return json.dumps({'targets': [{'package': package, 'platform': 'linux'} for package in packages]}) + + git('init', '-b', 'main') + git('config', 'user.name', 'CI Test') + git('config', 'user.email', 'ci@example.invalid') + path = '.github/integration-tests.json' + original = commit('README.md', 'fixture') + head = commit(path, manifest('first')) + self.assertEqual(base_manifest_targets(root, original, head), []) + fork = commit(path, manifest('first', 'second')) + git('checkout', '-b', 'feature') + head = commit(path, manifest('first', 'second', 'third')) + git('checkout', 'main') + # The base branch edits the manifest after the PR forks; compare against the fork point. + base = commit(path, manifest('first', 'second', 'base_only')) + base_targets = base_manifest_targets(root, base, head) + self.assertEqual(base_targets, json.loads(manifest('first', 'second'))['targets']) + head_targets = json.loads(manifest('first', 'second', 'third'))['targets'] + self.assertEqual(select_targets(head_targets, [path], base_targets), head_targets[2:]) + self.assertEqual(fork, git('merge-base', base, head)) + def test_invalid_manifest_fails_instead_of_skipping(self): with tempfile.TemporaryDirectory() as directory: root = Path(directory) From 023bc9094dda099ef6118152986118bd2036f39b Mon Sep 17 00:00:00 2001 From: bin <17426470+boyan01@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:11:14 +0800 Subject: [PATCH 2/2] ci: select integration targets for pushes to main Pushes to main now diff github.event.before...github.sha with the same rules as pull requests instead of running every target. When the pushed base cannot be diffed (new branch with an all-zero SHA, or a force push whose previous tip is no longer in the history), the selector falls back to every target. A weekly schedule keeps a full regression run, and workflow_dispatch still runs every target. Only pull request runs share a concurrency group now: with narrowed push runs, a pending run on main that a later push replaced would leave its changes untested. --- .github/workflows/integration-tests.yml | 21 ++++++--- tool/ci/select_integration_targets.py | 22 ++++++++-- tool/ci/test_select_integration_targets.py | 50 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 473c447a..90593374 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -4,13 +4,17 @@ on: pull_request: push: branches: [main] + schedule: + # Weekly full regression run; pushes to main only run affected targets. + - cron: '23 19 * * 0' workflow_dispatch: permissions: contents: read concurrency: - group: integration-${{ github.event.pull_request.number || github.ref }} + # Only PR runs share a group: a replaced pending run on main would leave its pushed changes untested. + group: integration-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: @@ -30,13 +34,16 @@ jobs: id: targets env: EVENT: ${{ github.event_name }} - BASE: ${{ github.event.pull_request.base.sha }} - HEAD: ${{ github.event.pull_request.head.sha }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + PUSH_BEFORE: ${{ github.event.before }} run: | - args=() - if [[ "$EVENT" == "pull_request" ]]; then - args+=(--base "$BASE" --head "$HEAD") - fi + case "$EVENT" in + pull_request) args=(--base "$PR_BASE" --head "$PR_HEAD") ;; + push) args=(--base "$PUSH_BEFORE" --head "$GITHUB_SHA") ;; + # schedule and workflow_dispatch run every target. + *) args=() ;; + esac python3 -B tool/ci/select_integration_targets.py "${args[@]}" --output "$GITHUB_OUTPUT" integration: diff --git a/tool/ci/select_integration_targets.py b/tool/ci/select_integration_targets.py index fa76bdcf..39694a00 100644 --- a/tool/ci/select_integration_targets.py +++ b/tool/ci/select_integration_targets.py @@ -5,6 +5,7 @@ from pathlib import Path import re import subprocess +import sys ROOT = Path(__file__).resolve().parents[2] @@ -61,6 +62,16 @@ def load_targets(root): return targets +def diffable_base(root, base): + """Return base when it names a local commit, or None when a push diff cannot be computed. + + A new branch reports an all-zero base, and a force push can report a base + that is no longer in the history. + """ + exists = subprocess.run(['git', 'cat-file', '-e', f'{base}^{{commit}}'], cwd=root, capture_output=True) + return base if exists.returncode == 0 else None + + def changed_files(root, base, head): # Include both sides of moves so moving a file out of a package still tests it. output = subprocess.check_output( @@ -147,13 +158,16 @@ def select_targets(targets, paths, base_targets=(), workspace=None): def main(): parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument('--base', help='PR base SHA; omit to select all targets') - parser.add_argument('--head', default='HEAD', help='PR head SHA') + parser.add_argument('--base', help='PR base or pushed-before SHA; omit to select all targets') + parser.add_argument('--head', default='HEAD', help='PR head or pushed SHA') parser.add_argument('--output', type=Path, help='GitHub Actions output file') args = parser.parse_args() targets = load_targets(ROOT) - paths = changed_files(ROOT, args.base, args.head) if args.base else None - base_targets = base_manifest_targets(ROOT, args.base, args.head) if args.base else [] + base = args.base and diffable_base(ROOT, args.base) + if args.base and not base: + print(f'Cannot diff from {args.base}; selecting all targets.', file=sys.stderr) + paths = changed_files(ROOT, base, args.head) if base else None + base_targets = base_manifest_targets(ROOT, base, args.head) if base else [] selected = select_targets(targets, paths, base_targets, load_workspace(ROOT)) matrix = json.dumps({'include': selected}, separators=(',', ':')) print(matrix) diff --git a/tool/ci/test_select_integration_targets.py b/tool/ci/test_select_integration_targets.py index 02d70d34..692bb0ec 100644 --- a/tool/ci/test_select_integration_targets.py +++ b/tool/ci/test_select_integration_targets.py @@ -12,6 +12,7 @@ ROOT, base_manifest_targets, changed_files, + diffable_base, load_targets, load_workspace, read_pubspec, @@ -251,6 +252,55 @@ def manifest(*packages): self.assertEqual(select_targets(head_targets, [path], base_targets), head_targets[2:]) self.assertEqual(fork, git('merge-base', base, head)) + def test_push_base_falls_back_when_it_cannot_be_diffed(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + + def git(*args): + return subprocess.check_output(['git', *args], cwd=root, stderr=subprocess.DEVNULL).decode().strip() + + git('init', '-b', 'main') + git('config', 'user.name', 'CI Test') + git('config', 'user.email', 'ci@example.invalid') + (root / 'README.md').write_text('fixture') + git('add', '.') + git('commit', '-m', 'test fixture') + before = git('rev-parse', 'HEAD') + self.assertEqual(diffable_base(root, before), before) + # New branch. + self.assertIsNone(diffable_base(root, '0' * 40)) + # Force push that dropped the previous tip from the history. + self.assertIsNone(diffable_base(root, 'f' * 40)) + self.assertIsNone(diffable_base(root, git('rev-parse', 'HEAD^{tree}'))) + + def test_workflow_selection_arguments(self): + workflow = (ROOT / '.github/workflows/integration-tests.yml').read_text() + script = workflow.split(' - name: Select targets\n', 1)[1].split(' run: |\n', 1)[1] + script = '\n'.join(line[10:] for line in script.split('\n\n', 1)[0].splitlines()) + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + selector = root / 'tool/ci/select_integration_targets.py' + selector.parent.mkdir(parents=True) + selector.write_text('import sys\nprint(" ".join(sys.argv[1:]))\n') + env = { + **os.environ, + 'PR_BASE': 'pr-base', + 'PR_HEAD': 'pr-head', + 'PUSH_BEFORE': 'push-before', + 'GITHUB_SHA': 'pushed', + 'GITHUB_OUTPUT': 'output', + } + cases = { + 'pull_request': '--base pr-base --head pr-head --output output', + 'push': '--base push-before --head pushed --output output', + 'schedule': '--output output', + 'workflow_dispatch': '--output output', + } + for event, expected in cases.items(): + with self.subTest(event=event): + output = subprocess.check_output(['bash', '-e', '-c', script], cwd=root, env={**env, 'EVENT': event}) + self.assertEqual(output.decode().strip(), expected) + def test_invalid_manifest_fails_instead_of_skipping(self): with tempfile.TemporaryDirectory() as directory: root = Path(directory)