From 69f6d2e4debabfd8676209ca222b3bed4ce3834d Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:36:31 +0200 Subject: [PATCH 01/32] Test --- .github/workflows/health_base.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 3bbb3890..8333bc12 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -44,8 +44,8 @@ on: default: false type: boolean required: false - use-flutter: - description: Whether to setup Flutter in this workflow. + flutter_packages: + description: List of packages depending on Flutter. default: false required: false type: boolean @@ -100,15 +100,17 @@ jobs: - run: mkdir -p current_repo/output/ - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - if: ${{ inputs.use-flutter }} + if: ${{ inputs.flutter_packages != '' }} with: channel: ${{ inputs.sdk }} - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 - if: ${{ !inputs.use-flutter }} with: sdk: ${{ inputs.sdk }} + - run: ${FLUTTER_RUNNER_TOOL_CACHE}/flutter/bin/dart --version + - run: which dart + - name: Install coverage run: dart pub global activate coverage if: ${{ inputs.check == 'coverage' }} @@ -139,6 +141,7 @@ jobs: ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} \ --fail_on ${{ inputs.fail_on }} \ --warn_on ${{ inputs.warn_on }} \ + --flutter_packages ${{ inputs.flutter_packages }} \ --ignore_license ${{ inputs.ignore_license }} \ --ignore_coverage ${{ inputs.ignore_coverage }} \ --ignore_packages ${{ inputs.ignore_packages }} \ From 64eacbf36121bcdc7bf39ca91e4c169b26faab46 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:38:15 +0200 Subject: [PATCH 02/32] Update health --- .github/workflows/health.yaml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index efc2a7b8..8a87146a 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -26,7 +26,7 @@ name: Health # warn_on: "license,coverage,breaking,leaking" # coverage_web: false # upload_coverage: false -# use-flutter: true +# flutter_packages: true # ignore_license: "**.g.dart" # ignore_coverage: "**.mock.dart,**.g.dart" # ignore_packages: "pkgs/helper_package" @@ -81,7 +81,7 @@ on: default: false type: boolean required: false - use-flutter: + flutter_packages: description: Whether to setup Flutter in this workflow. default: false required: false @@ -122,7 +122,7 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -135,7 +135,7 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_license: ${{ inputs.ignore_license }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -151,7 +151,7 @@ jobs: upload_coverage: ${{ inputs.upload_coverage }} coverage_web: ${{ inputs.coverage_web }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_coverage: ${{ inputs.ignore_coverage }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -166,7 +166,7 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -179,7 +179,7 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -192,7 +192,7 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} + flutter_packages: ${{ inputs.flutter_packages }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} From 725ce1527a578fd4634b0e1090384b0022e26740 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:39:14 +0200 Subject: [PATCH 03/32] always flutter --- .github/workflows/health_base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 8333bc12..461f5245 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -100,7 +100,7 @@ jobs: - run: mkdir -p current_repo/output/ - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - if: ${{ inputs.flutter_packages != '' }} + # if: ${{ inputs.flutter_packages != '' }} with: channel: ${{ inputs.sdk }} From a143250a4f2b169eef9cbff6f4898e96a87dd084 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:43:51 +0200 Subject: [PATCH 04/32] test --- .github/workflows/health_base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 461f5245..4085ae30 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -108,8 +108,8 @@ jobs: with: sdk: ${{ inputs.sdk }} - - run: ${FLUTTER_RUNNER_TOOL_CACHE}/flutter/bin/dart --version - run: which dart + - run: ${{env.FLUTTER_RUNNER_TOOL_CACHE}}/flutter/bin/dart --version - name: Install coverage run: dart pub global activate coverage From fd0bf9b44fc4b3780855eed4bca49729e2ce05d6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:45:29 +0200 Subject: [PATCH 05/32] test --- .github/workflows/health_base.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 4085ae30..3289269b 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -109,6 +109,9 @@ jobs: sdk: ${{ inputs.sdk }} - run: which dart + - run: which flutter + - run: whereis dart + - run: whereis flutter - run: ${{env.FLUTTER_RUNNER_TOOL_CACHE}}/flutter/bin/dart --version - name: Install coverage From 1dc88f1bd6d5879035d50501a5bd3b8b35eb4242 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 13:50:09 +0200 Subject: [PATCH 06/32] test --- .github/workflows/health_base.yaml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 3289269b..5a3a30e6 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -98,21 +98,23 @@ jobs: if: ${{ inputs.check == 'coverage' }} || ${{ inputs.check == 'breaking' }} - run: mkdir -p current_repo/output/ + + - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 + with: + sdk: ${{ inputs.sdk }} + + - run: which dart + - run: whereis dart - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 # if: ${{ inputs.flutter_packages != '' }} with: channel: ${{ inputs.sdk }} - - - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 - with: - sdk: ${{ inputs.sdk }} - run: which dart - run: which flutter - run: whereis dart - run: whereis flutter - - run: ${{env.FLUTTER_RUNNER_TOOL_CACHE}}/flutter/bin/dart --version - name: Install coverage run: dart pub global activate coverage From 3b82710b96142c9b58322ac1e459c1ff2d07829f Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:36:25 +0200 Subject: [PATCH 07/32] Check all files --- pkgs/firehose/bin/health.dart | 7 ++ pkgs/firehose/lib/firehose.dart | 2 +- pkgs/firehose/lib/src/health/changelog.dart | 2 +- pkgs/firehose/lib/src/health/coverage.dart | 7 +- pkgs/firehose/lib/src/health/health.dart | 73 +++++++++++---- pkgs/firehose/lib/src/repo.dart | 13 ++- pkgs/firehose/test/health_test.dart | 88 ++++++++++++------- .../golden/comment_breaking_healthchanged.md | 16 ++++ .../golden/comment_changelog_healthchanged.md | 16 ++++ .../golden/comment_coverage_healthchanged.md | 23 +++++ .../comment_do-not-submit_healthchanged.md | 15 ++++ .../golden/comment_leaking_healthchanged.md | 15 ++++ .../golden/comment_license_healthchanged.md | 34 +++++++ 13 files changed, 254 insertions(+), 57 deletions(-) create mode 100644 pkgs/firehose/test_data/golden/comment_breaking_healthchanged.md create mode 100644 pkgs/firehose/test_data/golden/comment_changelog_healthchanged.md create mode 100644 pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md create mode 100644 pkgs/firehose/test_data/golden/comment_do-not-submit_healthchanged.md create mode 100644 pkgs/firehose/test_data/golden/comment_leaking_healthchanged.md create mode 100644 pkgs/firehose/test_data/golden/comment_license_healthchanged.md diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 697dd74c..e86e6278 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -48,12 +48,18 @@ void main(List arguments) async { ..addFlag( 'coverage_web', help: 'Whether to run web tests for coverage', + ) + ..addMultiOption( + 'flutter_packages', + defaultsTo: [], + help: 'The Flutter packages in this repo', ); final parsedArgs = argParser.parse(arguments); final checkStr = parsedArgs['check'] as String; final check = Check.values.firstWhere((c) => c.name == checkStr); final warnOn = parsedArgs['warn_on'] as List; final failOn = parsedArgs['fail_on'] as List; + final flutterPackages = parsedArgs.multiOption('flutter_packages'); final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages'); final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license'); final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage'); @@ -74,6 +80,7 @@ void main(List arguments) async { ignoreCoverage, experiments, GithubApi(), + flutterPackages, ).healthCheck(); } diff --git a/pkgs/firehose/lib/firehose.dart b/pkgs/firehose/lib/firehose.dart index 618dbb6f..f7d7944b 100644 --- a/pkgs/firehose/lib/firehose.dart +++ b/pkgs/firehose/lib/firehose.dart @@ -100,7 +100,7 @@ Saving existing comment id $existingCommentId to file ${idFile.path}'''); Future verify(GithubApi github) async { var repo = Repository(directory); - var packages = repo.locatePackages(ignoredPackages); + var packages = repo.locatePackages(ignore: ignoredPackages); var pub = Pub(); diff --git a/pkgs/firehose/lib/src/health/changelog.dart b/pkgs/firehose/lib/src/health/changelog.dart index 95ad14ab..e725ff5a 100644 --- a/pkgs/firehose/lib/src/health/changelog.dart +++ b/pkgs/firehose/lib/src/health/changelog.dart @@ -16,7 +16,7 @@ Future>> packagesWithoutChangelog( Directory directory, ) async { final repo = Repository(directory); - final packages = repo.locatePackages(ignoredPackages); + final packages = repo.locatePackages(ignore: ignoredPackages); final files = await github.listFilesForPR(directory, ignoredPackages); diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index bb0610dc..5f6681e1 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -30,14 +30,13 @@ class Coverage { ); Future compareCoverages( - GithubApi github, Directory base) async { - var files = await github.listFilesForPR(directory, ignoredFiles); + List files, Directory base) async { return compareCoveragesFor(files, base); } CoverageResult compareCoveragesFor(List files, Directory base) { var repository = Repository(directory); - var packages = repository.locatePackages(ignoredPackages); + var packages = repository.locatePackages(ignore: ignoredPackages); print('Found packages $packages at $directory'); var filesOfInterest = files @@ -49,7 +48,7 @@ class Coverage { print('The files of interest are $filesOfInterest'); var baseRepository = Repository(base); - var basePackages = baseRepository.locatePackages(ignoredFiles); + var basePackages = baseRepository.locatePackages(ignore: ignoredFiles); print('Found packages $basePackages at $base'); var changedPackages = packages diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 8850519d..34fdec72 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -47,13 +47,17 @@ class Health { List ignoredLicense, List ignoredCoverage, this.experiments, - this.github, { + this.github, + List flutterPackages, { Directory? base, String? comment, this.log = printLogger, }) : ignoredPackages = ignoredPackages .map((pattern) => Glob(pattern, recursive: true)) .toList(), + flutterPackages = flutterPackages + .map((pattern) => Glob(pattern, recursive: true)) + .toList(), ignoredFilesForCoverage = ignoredCoverage .map((pattern) => Glob(pattern, recursive: true)) .toList(), @@ -76,6 +80,7 @@ class Health { final List ignoredPackages; final List ignoredFilesForLicense; final List ignoredFilesForCoverage; + final List flutterPackages; final Directory baseDirectory; final List experiments; final Logger log; @@ -127,9 +132,12 @@ class Health { }; Future breakingCheck() async { - final filesInPR = await github.listFilesForPR(directory, ignoredPackages); + final filesInPR = await listFilesInPRorAll(); final changeForPackage = {}; - for (var package in packagesContaining(filesInPR, ignoredPackages)) { + final flutter = packagesContaining(filesInPR, only: flutterPackages); + + for (var package + in packagesContaining(filesInPR, ignore: ignoredPackages)) { log('Look for changes in $package with base $baseDirectory'); var relativePath = path.relative(package.directory.path, from: directory.path); @@ -144,7 +152,7 @@ class Health { ...['pub', 'global', 'run'], 'dart_apitool:main', 'diff', - '--no-check-sdk-version', + if (flutter.contains(package)) '--force-use-flutter', ...['--old', baseRelativePath], ...['--new', relativePath], ...['--report-format', 'json'], @@ -200,9 +208,10 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} } Future leakingCheck() async { - final filesInPR = await github.listFilesForPR(directory, ignoredPackages); + var filesInPR = await listFilesInPRorAll(); final leaksForPackage = >{}; - for (var package in packagesContaining(filesInPR, ignoredPackages)) { + for (var package + in packagesContaining(filesInPR, ignore: ignoredPackages)) { log('Look for leaks in $package'); var relativePath = path.relative(package.directory.path, from: directory.path); @@ -248,7 +257,7 @@ ${leaksForPackage.entries.map((e) => '|${e.key.name}|${e.value.join('
')}|'). } Future licenseCheck() async { - var files = await github.listFilesForPR(directory, ignoredPackages); + var files = await listFilesInPRorAll(); var allFilePaths = await getFilesWithoutLicenses( directory, [...ignoredFilesForLicense, ...ignoredPackages], @@ -294,6 +303,26 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} ); } + Future> getAllFiles() async { + return await directory + .list(recursive: true) + .where((entity) => entity is File) + .where( + (file) => ignoredPackages.none((glob) => glob.matches(file.path))) + .map((file) => path.relative(file.path, from: directory.path)) + .map((file) => GitFile(file, FileStatus.added, directory)) + .toList(); + } + + bool healthYamlChanged(List files) { + return files + .where((file) => + [FileStatus.added, FileStatus.modified].contains(file.status)) + .any((file) => + file.filename.endsWith('health.yaml') || + file.filename.endsWith('health.yml')); + } + Future changelogCheck() async { var filePaths = await packagesWithoutChangelog( github, @@ -322,7 +351,7 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst const supportedExtensions = ['.dart', '.json', '.md', '.txt']; final body = await github.pullrequestBody(); - final files = await github.listFilesForPR(directory, ignoredPackages); + var files = await listFilesInPRorAll(); log('Checking for DO_NOT${'_'}SUBMIT strings: $files'); final filesWithDNS = files .where((file) => @@ -353,26 +382,37 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ); } + Future> listFilesInPRorAll() async { + var files = await github.listFilesForPR(directory, ignoredPackages); + if (healthYamlChanged(files)) { + files = await getAllFiles(); + } + return files; + } + Future coverageCheck() async { - var coverage = await Coverage( + var coverage = Coverage( coverageweb, ignoredFilesForCoverage, ignoredPackages, directory, experiments, - ).compareCoverages(github, directory); + ); + + var files = await listFilesInPRorAll(); + var coverageResult = await coverage.compareCoverages(files, directory); var markdownResult = ''' | File | Coverage | | :--- | :--- | -${coverage.coveragePerFile.entries.map((e) => '|${e.key}| ${e.value.toMarkdown()} |').join('\n')} +${coverageResult.coveragePerFile.entries.map((e) => '|${e.key}| ${e.value.toMarkdown()} |').join('\n')} This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test-Coverage) is informational (issues shown here will not fail the PR). '''; return HealthCheckResult( Check.coverage, - Severity.values[coverage.coveragePerFile.values + Severity.values[coverageResult.coveragePerFile.values .map((change) => change.severity.index) .fold(0, max)], markdownResult, @@ -415,12 +455,13 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } List packagesContaining( - List filesInPR, - List ignoredPackages, - ) { + List filesInPR, { + List? ignore, + List? only, + }) { var files = filesInPR.where((element) => element.status.isRelevant); return Repository(directory) - .locatePackages(ignoredPackages) + .locatePackages(ignore: ignore, only: only) .where((package) => files.any((file) => path.isWithin(package.directory.path, file.pathInRepository))) .toList(); diff --git a/pkgs/firehose/lib/src/repo.dart b/pkgs/firehose/lib/src/repo.dart index 1d5fea11..3e174067 100644 --- a/pkgs/firehose/lib/src/repo.dart +++ b/pkgs/firehose/lib/src/repo.dart @@ -4,6 +4,7 @@ import 'dart:io'; +import 'package:collection/collection.dart'; import 'package:glob/glob.dart'; import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; @@ -40,11 +41,17 @@ class Repository { /// `publish_to: none` key. /// /// Once we find a package, we don't look for packages in sub-directories. - List locatePackages([List ignore = const []]) { + List locatePackages({List? only, List? ignore}) { final packages = []; _recurseAndGather(baseDirectory, packages); - packages.removeWhere((package) => ignore.any((glob) => glob.matches( - path.relative(package.directory.path, from: baseDirectory.path)))); + if (ignore != null) { + packages.removeWhere((package) => ignore.any((glob) => glob.matches( + path.relative(package.directory.path, from: baseDirectory.path)))); + } + if (only != null) { + packages.removeWhere((package) => only.none((glob) => glob.matches( + path.relative(package.directory.path, from: baseDirectory.path)))); + } packages.sort((a, b) => a.name.compareTo(b.name)); return packages; } diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart index 455331bd..6e1ce2b2 100644 --- a/pkgs/firehose/test/health_test.dart +++ b/pkgs/firehose/test/health_test.dart @@ -14,37 +14,39 @@ import 'package:test/test.dart'; Future main() async { late final Directory directory; - late final FakeGithubApi fakeGithubApi; + late final FakeGithubApi Function(List additional) fakeGithubApi; setUpAll(() async { directory = Directory(p.join('test_data', 'test_repo')); - fakeGithubApi = FakeGithubApi(prLabels: [], files: [ - GitFile( - 'pkgs/package1/bin/package1.dart', - FileStatus.modified, - directory, - ), - GitFile( - 'pkgs/package2/lib/anotherLib.dart', - FileStatus.added, - directory, - ), - GitFile( - 'pkgs/package2/someImage.png', - FileStatus.added, - directory, - ), - GitFile( - 'pkgs/package5/lib/src/package5_base.dart', - FileStatus.modified, - directory, - ), - GitFile( - 'pkgs/package5/pubspec.yaml', - FileStatus.modified, - directory, - ), - ]); + fakeGithubApi = + (List additional) => FakeGithubApi(prLabels: [], files: [ + GitFile( + 'pkgs/package1/bin/package1.dart', + FileStatus.modified, + directory, + ), + GitFile( + 'pkgs/package2/lib/anotherLib.dart', + FileStatus.added, + directory, + ), + GitFile( + 'pkgs/package2/someImage.png', + FileStatus.added, + directory, + ), + GitFile( + 'pkgs/package5/lib/src/package5_base.dart', + FileStatus.modified, + directory, + ), + GitFile( + 'pkgs/package5/pubspec.yaml', + FileStatus.modified, + directory, + ), + ...additional + ]); await Process.run('dart', ['pub', 'global', 'activate', 'dart_apitool']); await Process.run('dart', ['pub', 'global', 'activate', 'coverage']); @@ -53,7 +55,26 @@ Future main() async { for (var check in Check.values) { test( 'Check health workflow "${check.name}" against golden files', - () async => await checkGolden(check, fakeGithubApi, directory), + () async => await checkGolden( + check, + fakeGithubApi([]), + directory, + ), + timeout: const Timeout(Duration(minutes: 2)), + ); + test( + 'Check health workflow "${check.name}" against golden files', + () async => await checkGolden( + check, + fakeGithubApi([ + GitFile( + '.github/workflows/health.yaml', + FileStatus.added, + directory, + ), + ]), + directory, + suffix: '_healthchanged'), timeout: const Timeout(Duration(minutes: 2)), ); } @@ -61,7 +82,7 @@ Future main() async { test('Ignore license test', () async { await checkGolden( Check.license, - fakeGithubApi, + fakeGithubApi([]), directory, suffix: '_ignore_license', ignoredLicense: ['pkgs/package3/**'], @@ -71,10 +92,11 @@ Future main() async { test( 'Ignore packages test', () async { - for (var check in Check.values) { + for (var check + in Check.values.whereNot((element) => element == Check.coverage)) { await checkGolden( check, - fakeGithubApi, + fakeGithubApi([]), directory, suffix: '_ignore_package', ignoredPackage: ['pkgs/package1'], @@ -92,6 +114,7 @@ Future checkGolden( String suffix = '', List ignoredLicense = const [], List ignoredPackage = const [], + List flutterPackages = const [], }) async { final commentPath = p.join( Directory.systemTemp.createTempSync().path, 'comment_${check.name}.md'); @@ -106,6 +129,7 @@ Future checkGolden( [], [], fakeGithubApi, + flutterPackages, base: Directory(p.join('test_data', 'base_test_repo')), comment: commentPath, log: printOnFailure, diff --git a/pkgs/firehose/test_data/golden/comment_breaking_healthchanged.md b/pkgs/firehose/test_data/golden/comment_breaking_healthchanged.md new file mode 100644 index 00000000..62ca9598 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_breaking_healthchanged.md @@ -0,0 +1,16 @@ +
+ +Breaking changes :warning: + + +| Package | Change | Current Version | New Version | Needed Version | Looking good? | +| :--- | :--- | ---: | ---: | ---: | ---: | +|package1|None|1.0.0|1.0.0|1.0.0|:heavy_check_mark:| +|package2|Non-Breaking|1.0.0|1.0.0|**1.1.0**
Got "1.0.0" expected >= "1.1.0" (non-breaking changes)|:warning:| +|package3|None|1.0.0|1.0.0|1.0.0|:heavy_check_mark:| +|package5|Non-Breaking|1.0.0|1.2.0|1.1.0|:heavy_check_mark:| + + +This check can be disabled by tagging the PR with `skip-breaking-check`. +
+ diff --git a/pkgs/firehose/test_data/golden/comment_changelog_healthchanged.md b/pkgs/firehose/test_data/golden/comment_changelog_healthchanged.md new file mode 100644 index 00000000..ae601aeb --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_changelog_healthchanged.md @@ -0,0 +1,16 @@ +
+ +Changelog Entry :exclamation: + + +| Package | Changed Files | +| :--- | :--- | +| package:package1 | pkgs/package1/bin/package1.dart | +| package:package2 | pkgs/package2/lib/anotherLib.dart | + +Changes to files need to be [accounted for](https://github.com/dart-lang/ecosystem/wiki/Changelog) in their respective changelogs. + + +This check can be disabled by tagging the PR with `skip-changelog-check`. +
+ diff --git a/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md b/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md new file mode 100644 index 00000000..6141d8d2 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md @@ -0,0 +1,23 @@ +
+ +Coverage :warning: + + +| File | Coverage | +| :--- | :--- | +|pkgs/package1/bin/package1.dart| :broken_heart: Not covered | +|pkgs/package1/lib/package1.dart| :green_heart: 100 % | +|pkgs/package2/bin/package2.dart| :broken_heart: Not covered | +|pkgs/package2/lib/anotherLib.dart| :green_heart: 100 % | +|pkgs/package2/lib/package2.dart| :green_heart: 100 % | +|pkgs/package3/bin/package3.dart| :broken_heart: Not covered | +|pkgs/package3/lib/package3.dart| :broken_heart: Not covered | +|pkgs/package5/lib/package5.dart| :broken_heart: Not covered | +|pkgs/package5/lib/src/package5_base.dart| :broken_heart: Not covered | + +This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test-Coverage) is informational (issues shown here will not fail the PR). + + +This check can be disabled by tagging the PR with `skip-coverage-check`. +
+ diff --git a/pkgs/firehose/test_data/golden/comment_do-not-submit_healthchanged.md b/pkgs/firehose/test_data/golden/comment_do-not-submit_healthchanged.md new file mode 100644 index 00000000..530c94f3 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_do-not-submit_healthchanged.md @@ -0,0 +1,15 @@ +
+ +Do Not Submit :exclamation: + + +Body contains `DO_NOT_SUBMIT`: false + +| Files with `DO_NOT_SUBMIT` | +| :--- | +|pkgs/package1/bin/package1.dart| + + +This check can be disabled by tagging the PR with `skip-do-not-submit-check`. +
+ diff --git a/pkgs/firehose/test_data/golden/comment_leaking_healthchanged.md b/pkgs/firehose/test_data/golden/comment_leaking_healthchanged.md new file mode 100644 index 00000000..b0abc3d2 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_leaking_healthchanged.md @@ -0,0 +1,15 @@ +
+ +API leaks :warning: + + +The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API. + +| Package | Leaked API symbols | +| :--- | :--- | +|package5|NonExported
NonExported2
TransitiveNonExported| + + +This check can be disabled by tagging the PR with `skip-leaking-check`. +
+ diff --git a/pkgs/firehose/test_data/golden/comment_license_healthchanged.md b/pkgs/firehose/test_data/golden/comment_license_healthchanged.md new file mode 100644 index 00000000..16e04cb8 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_license_healthchanged.md @@ -0,0 +1,34 @@ +
+ +License Headers :exclamation: + + +``` +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +``` + +| Files | +| :--- | +|pkgs/package1/bin/package1.dart| +|pkgs/package1/lib/package1.dart| +|pkgs/package1/test/package1_test.dart| +|pkgs/package2/lib/anotherLib.dart| +|pkgs/package2/lib/package2.dart| +|pkgs/package2/test/package2_test.dart| +|pkgs/package3/bin/package3.dart| +|pkgs/package3/lib/package3.dart| +|pkgs/package3/test/package3_test.dart| +|pkgs/package5/lib/package5.dart| +|pkgs/package5/lib/src/package5_base.dart| + +All source files should start with a [license header](https://github.com/dart-lang/ecosystem/wiki/License-Header). + + + + + +This check can be disabled by tagging the PR with `skip-license-check`. +
+ From c82fa4f146bc0d659776399cce6c1d3fb4cac97c Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:38:53 +0200 Subject: [PATCH 08/32] fixes --- .github/workflows/health.yaml | 2 +- .github/workflows/health_base.yaml | 10 +--------- pkgs/firehose/bin/health.dart | 8 ++++---- pkgs/firehose/test/health_test.dart | 3 +-- 4 files changed, 7 insertions(+), 16 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 8a87146a..72303465 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -26,7 +26,7 @@ name: Health # warn_on: "license,coverage,breaking,leaking" # coverage_web: false # upload_coverage: false -# flutter_packages: true +# flutter_packages: "pkgs/my_flutter_package" # ignore_license: "**.g.dart" # ignore_coverage: "**.mock.dart,**.g.dart" # ignore_packages: "pkgs/helper_package" diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 5a3a30e6..ab494863 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -103,19 +103,11 @@ jobs: with: sdk: ${{ inputs.sdk }} - - run: which dart - - run: whereis dart - - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - # if: ${{ inputs.flutter_packages != '' }} + if: ${{ inputs.flutter_packages != '' }} with: channel: ${{ inputs.sdk }} - - run: which dart - - run: which flutter - - run: whereis dart - - run: whereis flutter - - name: Install coverage run: dart pub global activate coverage if: ${{ inputs.check == 'coverage' }} diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index e86e6278..7e2a2e8c 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -55,16 +55,16 @@ void main(List arguments) async { help: 'The Flutter packages in this repo', ); final parsedArgs = argParser.parse(arguments); - final checkStr = parsedArgs['check'] as String; + final checkStr = parsedArgs.option('check'); final check = Check.values.firstWhere((c) => c.name == checkStr); - final warnOn = parsedArgs['warn_on'] as List; - final failOn = parsedArgs['fail_on'] as List; + final warnOn = parsedArgs.multiOption('warn_on'); + final failOn = parsedArgs.multiOption('fail_on'); final flutterPackages = parsedArgs.multiOption('flutter_packages'); final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages'); final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license'); final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage'); final experiments = _listNonEmpty(parsedArgs, 'experiments'); - final coverageWeb = parsedArgs['coverage_web'] as bool; + final coverageWeb = parsedArgs.flag('coverage_web'); if (warnOn.toSet().intersection(failOn.toSet()).isNotEmpty) { throw ArgumentError('The checks for which warnings are displayed and the ' 'checks which lead to failure must be disjoint.'); diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart index 6e1ce2b2..a30e65d3 100644 --- a/pkgs/firehose/test/health_test.dart +++ b/pkgs/firehose/test/health_test.dart @@ -92,8 +92,7 @@ Future main() async { test( 'Ignore packages test', () async { - for (var check - in Check.values.whereNot((element) => element == Check.coverage)) { + for (var check in Check.values) { await checkGolden( check, fakeGithubApi([]), From 3155fec63123698fb057436192cbaa9c585c5faf Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:42:54 +0200 Subject: [PATCH 09/32] Use channel and sdk --- .github/workflows/health_base.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index ab494863..dcae63c5 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -15,6 +15,14 @@ on: default: "stable" required: false type: string + channel: + description: >- + The channel, or a specific version from a channel, to install + ('2.19.0','stable', 'beta', 'dev'). Using one of the three channels + will give you the latest version published to that channel. + default: "stable" + required: false + type: string check: description: What to check for in the PR health check - any of "changelog,license,coverage,breaking,do-not-submit,leaking" type: string @@ -106,7 +114,7 @@ jobs: - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 if: ${{ inputs.flutter_packages != '' }} with: - channel: ${{ inputs.sdk }} + channel: ${{ inputs.channel }} - name: Install coverage run: dart pub global activate coverage From cbdadc340f5ea523cceda0e98eed5a6f407a5a26 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:46:43 +0200 Subject: [PATCH 10/32] use in health --- .github/workflows/health.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 72303465..634eb02f 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -49,6 +49,14 @@ on: default: "stable" required: false type: string + channel: + description: >- + The channel, or a specific version from a channel, to install + ('2.19.0','stable', 'beta', 'dev'). Using one of the three channels + will give you the latest version published to that channel. + default: "stable" + required: false + type: string # Restrict the checks to any subset of version, changelog, and license if # needed. checks: From 8656647e8d319ee53aa142c08438b47845125565 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:49:31 +0200 Subject: [PATCH 11/32] change type --- .github/workflows/health.yaml | 6 +++--- .github/workflows/health_base.yaml | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 634eb02f..1f8d6824 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -90,10 +90,10 @@ on: type: boolean required: false flutter_packages: - description: Whether to setup Flutter in this workflow. - default: false + description: List of packages depending on Flutter. + default: '' required: false - type: boolean + type: string ignore_license: description: Which files to ignore for the license check. default: "\"\"" diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index dcae63c5..635706c5 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -54,9 +54,9 @@ on: required: false flutter_packages: description: List of packages depending on Flutter. - default: false + default: '' required: false - type: boolean + type: string ignore_license: description: Which files to ignore for the license check. default: "\"\"" From 8f3a03ddc8a9d85faefed6f239f3d45ea37c987e Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 15:54:56 +0200 Subject: [PATCH 12/32] Use current firehose --- .github/workflows/health_base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 635706c5..5833fa3e 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -121,7 +121,7 @@ jobs: if: ${{ inputs.check == 'coverage' }} - name: Install firehose - run: dart pub global activate firehose + run: dart pub global activate --source git https://github.com/dart-lang/ecosystem --git-path pkgs/firehose/ --git-ref=forceFlutter if: ${{ !inputs.local_debug }} - name: Install local firehose From 67bf2dedf314cbcaba797bdd79987e8cf9127f9f Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:03:01 +0200 Subject: [PATCH 13/32] Change exception --- pkgs/firehose/lib/src/health/health.dart | 36 +++++++++++++++--------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 34fdec72..9e47e8ec 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -217,28 +217,36 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} path.relative(package.directory.path, from: directory.path); var tempDirectory = Directory.systemTemp.createTempSync(); var reportPath = path.join(tempDirectory.path, 'leaks.json'); + var arguments = [ + ...['pub', 'global', 'run'], + 'dart_apitool:main', + 'extract', + ...['--input', relativePath], + ...['--output', reportPath], + '--set-exit-on-missing-export', + ]; var runApiTool = Process.runSync( 'dart', - [ - ...['pub', 'global', 'run'], - 'dart_apitool:main', - 'extract', - ...['--input', relativePath], - ...['--output', reportPath], - '--set-exit-on-missing-export', - ], + arguments, workingDirectory: directory.path, ); log(runApiTool.stderr as String); log(runApiTool.stdout as String); - var fullReportString = File(reportPath).readAsStringSync(); - var decoded = jsonDecode(fullReportString) as Map; - var leaks = decoded['missingEntryPoints'] as List; + if (runApiTool.exitCode == 0) { + var fullReportString = await File(reportPath).readAsString(); + var decoded = jsonDecode(fullReportString) as Map; + var leaks = decoded['missingEntryPoints'] as List; - log('Leaking symbols in API:\n$leaks'); - if (leaks.isNotEmpty) { - leaksForPackage[package] = leaks.cast(); + log('Leaking symbols in API:\n$leaks'); + if (leaks.isNotEmpty) { + leaksForPackage[package] = leaks.cast(); + } + } else { + throw ProcessException( + 'Api tool finished with exit code ${runApiTool.exitCode}', + arguments, + ); } } return HealthCheckResult( From 3519e44ca1b4d95390902ba2777d74b06cdebec5 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:03:38 +0200 Subject: [PATCH 14/32] do not set exitcode --- pkgs/firehose/lib/src/health/health.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 9e47e8ec..e95ca2a6 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -223,7 +223,6 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} 'extract', ...['--input', relativePath], ...['--output', reportPath], - '--set-exit-on-missing-export', ]; var runApiTool = Process.runSync( 'dart', From 2ebafb53ad8b0419acf3ed651559bb1ae09cf25f Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:16:36 +0200 Subject: [PATCH 15/32] Update --- .github/workflows/health_base.yaml | 1 + pkgs/firehose/lib/src/health/health.dart | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 5833fa3e..ce80de9a 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -122,6 +122,7 @@ jobs: - name: Install firehose run: dart pub global activate --source git https://github.com/dart-lang/ecosystem --git-path pkgs/firehose/ --git-ref=forceFlutter + # DO-NOT-SUBMIT if: ${{ !inputs.local_debug }} - name: Install local firehose diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index e95ca2a6..345cdcdf 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -314,9 +314,8 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} return await directory .list(recursive: true) .where((entity) => entity is File) - .where( - (file) => ignoredPackages.none((glob) => glob.matches(file.path))) .map((file) => path.relative(file.path, from: directory.path)) + .where((file) => ignoredPackages.none((glob) => glob.matches(file))) .map((file) => GitFile(file, FileStatus.added, directory)) .toList(); } From a12c2008a0868ba78752b1aa8c5f980498339f0e Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:23:45 +0200 Subject: [PATCH 16/32] Fixes --- .github/workflows/health.yaml | 2 +- pkgs/firehose/bin/health.dart | 2 +- pkgs/firehose/lib/src/health/health.dart | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 1f8d6824..240a32dc 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -91,7 +91,7 @@ on: required: false flutter_packages: description: List of packages depending on Flutter. - default: '' + default: "\"\"" required: false type: string ignore_license: diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 7e2a2e8c..122e0444 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -59,7 +59,7 @@ void main(List arguments) async { final check = Check.values.firstWhere((c) => c.name == checkStr); final warnOn = parsedArgs.multiOption('warn_on'); final failOn = parsedArgs.multiOption('fail_on'); - final flutterPackages = parsedArgs.multiOption('flutter_packages'); + final flutterPackages = _listNonEmpty(parsedArgs, 'flutter_packages'); final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages'); final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license'); final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage'); diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 345cdcdf..b29be0c0 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -96,6 +96,7 @@ class Health { log(' warnOn: $warnOn'); log(' failOn: $failOn'); log(' coverageweb: $coverageweb'); + log(' flutterPackages: $flutterPackages'); log(' ignoredPackages: $ignoredPackages'); log(' ignoredForLicense: $ignoredFilesForLicense'); log(' ignoredForCoverage: $ignoredFilesForCoverage'); @@ -389,9 +390,9 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} } Future> listFilesInPRorAll() async { - var files = await github.listFilesForPR(directory, ignoredPackages); + final files = await github.listFilesForPR(directory, ignoredPackages); if (healthYamlChanged(files)) { - files = await getAllFiles(); + return await getAllFiles(); } return files; } From e052ec96d442d10fb69b265b5c1eb4295e4fd593 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:35:21 +0200 Subject: [PATCH 17/32] add debug --- pkgs/firehose/lib/src/health/health.dart | 39 ++++++++++++------------ 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index b29be0c0..7471c4b5 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -133,7 +133,7 @@ class Health { }; Future breakingCheck() async { - final filesInPR = await listFilesInPRorAll(); + final filesInPR = await listFilesInPRorAll(ignoredPackages); final changeForPackage = {}; final flutter = packagesContaining(filesInPR, only: flutterPackages); @@ -209,10 +209,10 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} } Future leakingCheck() async { - var filesInPR = await listFilesInPRorAll(); + var filesInPR = await listFilesInPRorAll(ignoredPackages); + log('Files: $filesInPR'); final leaksForPackage = >{}; - for (var package - in packagesContaining(filesInPR, ignore: ignoredPackages)) { + for (var package in packagesContaining(filesInPR)) { log('Look for leaks in $package'); var relativePath = path.relative(package.directory.path, from: directory.path); @@ -265,7 +265,7 @@ ${leaksForPackage.entries.map((e) => '|${e.key.name}|${e.value.join('
')}|'). } Future licenseCheck() async { - var files = await listFilesInPRorAll(); + var files = await listFilesInPRorAll(ignoredPackages); var allFilePaths = await getFilesWithoutLicenses( directory, [...ignoredFilesForLicense, ...ignoredPackages], @@ -311,16 +311,6 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} ); } - Future> getAllFiles() async { - return await directory - .list(recursive: true) - .where((entity) => entity is File) - .map((file) => path.relative(file.path, from: directory.path)) - .where((file) => ignoredPackages.none((glob) => glob.matches(file))) - .map((file) => GitFile(file, FileStatus.added, directory)) - .toList(); - } - bool healthYamlChanged(List files) { return files .where((file) => @@ -358,7 +348,7 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst const supportedExtensions = ['.dart', '.json', '.md', '.txt']; final body = await github.pullrequestBody(); - var files = await listFilesInPRorAll(); + var files = await listFilesInPRorAll(ignoredPackages); log('Checking for DO_NOT${'_'}SUBMIT strings: $files'); final filesWithDNS = files .where((file) => @@ -389,14 +379,23 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ); } - Future> listFilesInPRorAll() async { - final files = await github.listFilesForPR(directory, ignoredPackages); + Future> listFilesInPRorAll(List ignore) async { + final files = await github.listFilesForPR(directory, ignore); if (healthYamlChanged(files)) { - return await getAllFiles(); + return await _getAllFiles(ignore); } return files; } + Future> _getAllFiles(List ignored) async => + await directory + .list(recursive: true) + .where((entity) => entity is File) + .map((file) => path.relative(file.path, from: directory.path)) + .where((file) => ignored.none((glob) => glob.matches(file))) + .map((file) => GitFile(file, FileStatus.added, directory)) + .toList(); + Future coverageCheck() async { var coverage = Coverage( coverageweb, @@ -406,7 +405,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} experiments, ); - var files = await listFilesInPRorAll(); + var files = await listFilesInPRorAll(ignoredPackages); var coverageResult = await coverage.compareCoverages(files, directory); var markdownResult = ''' From b2bccb1c0ca3f920de54f1b3dedfa0375b625cb9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:38:59 +0200 Subject: [PATCH 18/32] better log --- pkgs/firehose/lib/src/health/health.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 7471c4b5..839f652d 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -210,7 +210,7 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} Future leakingCheck() async { var filesInPR = await listFilesInPRorAll(ignoredPackages); - log('Files: $filesInPR'); + print('Files: ${filesInPR.map((e) => e.filename).join(', ')}'); final leaksForPackage = >{}; for (var package in packagesContaining(filesInPR)) { log('Look for leaks in $package'); From d39459d9401cf99f043c2f03123cfd1d94d9b628 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:46:53 +0200 Subject: [PATCH 19/32] more printing --- pkgs/firehose/lib/src/health/health.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 839f652d..97bb7f85 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -210,7 +210,10 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} Future leakingCheck() async { var filesInPR = await listFilesInPRorAll(ignoredPackages); + // DO-NOT-SUBMIT print('Files: ${filesInPR.map((e) => e.filename).join(', ')}'); + print( + 'Files: ${filesInPR.map((e) => e.filename).where((element) => element.contains('swiftgen')).join(', ')}'); final leaksForPackage = >{}; for (var package in packagesContaining(filesInPR)) { log('Look for leaks in $package'); From 123810b2e65b902d69f33d44b90709022cbcbf3d Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 16:48:27 +0200 Subject: [PATCH 20/32] more prints --- pkgs/firehose/lib/src/health/health.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 97bb7f85..35a38cab 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -53,6 +53,10 @@ class Health { String? comment, this.log = printLogger, }) : ignoredPackages = ignoredPackages + .map((e) { + print('IGNORED:PACKAGE $e'); + return e; + }) .map((pattern) => Glob(pattern, recursive: true)) .toList(), flutterPackages = flutterPackages From 75666ad24738466d853d9cdf756d0e32c0266dc3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 18:22:31 +0200 Subject: [PATCH 21/32] Prepare for review --- .github/workflows/health.yaml | 4 +- .github/workflows/health_base.yaml | 2 +- pkgs/firehose/lib/src/health/coverage.dart | 5 --- pkgs/firehose/lib/src/health/health.dart | 49 +++++++--------------- pkgs/firehose/test/health_test.dart | 4 +- 5 files changed, 21 insertions(+), 43 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 240a32dc..ace435e7 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -43,9 +43,7 @@ on: # stable release of the Dart SDK. sdk: description: >- - The channel, or a specific version from a channel, to install - ('2.19.0','stable', 'beta', 'dev'). Using one of the three channels - will give you the latest version published to that channel. + The Dart SDK version, either a semver or one of `dev`, `stable` etc. default: "stable" required: false type: string diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index ce80de9a..d3eaef42 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -54,7 +54,7 @@ on: required: false flutter_packages: description: List of packages depending on Flutter. - default: '' + default: "\"\"" required: false type: string ignore_license: diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index 5f6681e1..cb08daba 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -29,11 +29,6 @@ class Coverage { this.experiments, ); - Future compareCoverages( - List files, Directory base) async { - return compareCoveragesFor(files, base); - } - CoverageResult compareCoveragesFor(List files, Directory base) { var repository = Repository(directory); var packages = repository.locatePackages(ignore: ignoredPackages); diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 35a38cab..14440c0e 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -52,22 +52,10 @@ class Health { Directory? base, String? comment, this.log = printLogger, - }) : ignoredPackages = ignoredPackages - .map((e) { - print('IGNORED:PACKAGE $e'); - return e; - }) - .map((pattern) => Glob(pattern, recursive: true)) - .toList(), - flutterPackages = flutterPackages - .map((pattern) => Glob(pattern, recursive: true)) - .toList(), - ignoredFilesForCoverage = ignoredCoverage - .map((pattern) => Glob(pattern, recursive: true)) - .toList(), - ignoredFilesForLicense = ignoredLicense - .map((pattern) => Glob(pattern, recursive: true)) - .toList(), + }) : ignoredPackages = toGlobs(ignoredPackages), + flutterPackages = toGlobs(flutterPackages), + ignoredFilesForCoverage = toGlobs(ignoredCoverage), + ignoredFilesForLicense = toGlobs(ignoredLicense), baseDirectory = base ?? Directory('../base_repo'), commentPath = comment ?? path.join( @@ -75,6 +63,10 @@ class Health { 'output', 'comment.md', ); + + static List toGlobs(List ignoredPackages) => + ignoredPackages.map((pattern) => Glob(pattern, recursive: true)).toList(); + final GithubApi github; final Check check; @@ -214,10 +206,6 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} Future leakingCheck() async { var filesInPR = await listFilesInPRorAll(ignoredPackages); - // DO-NOT-SUBMIT - print('Files: ${filesInPR.map((e) => e.filename).join(', ')}'); - print( - 'Files: ${filesInPR.map((e) => e.filename).where((element) => element.contains('swiftgen')).join(', ')}'); final leaksForPackage = >{}; for (var package in packagesContaining(filesInPR)) { log('Look for leaks in $package'); @@ -318,14 +306,12 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} ); } - bool healthYamlChanged(List files) { - return files - .where((file) => - [FileStatus.added, FileStatus.modified].contains(file.status)) - .any((file) => - file.filename.endsWith('health.yaml') || - file.filename.endsWith('health.yml')); - } + bool healthYamlChanged(List files) => files + .where((file) => + [FileStatus.added, FileStatus.modified].contains(file.status)) + .any((file) => + file.filename.endsWith('health.yaml') || + file.filename.endsWith('health.yml')); Future changelogCheck() async { var filePaths = await packagesWithoutChangelog( @@ -388,10 +374,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} Future> listFilesInPRorAll(List ignore) async { final files = await github.listFilesForPR(directory, ignore); - if (healthYamlChanged(files)) { - return await _getAllFiles(ignore); - } - return files; + return healthYamlChanged(files) ? await _getAllFiles(ignore) : files; } Future> _getAllFiles(List ignored) async => @@ -413,7 +396,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ); var files = await listFilesInPRorAll(ignoredPackages); - var coverageResult = await coverage.compareCoverages(files, directory); + var coverageResult = coverage.compareCoveragesFor(files, directory); var markdownResult = ''' | File | Coverage | diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart index a30e65d3..0c830cd5 100644 --- a/pkgs/firehose/test/health_test.dart +++ b/pkgs/firehose/test/health_test.dart @@ -62,8 +62,10 @@ Future main() async { ), timeout: const Timeout(Duration(minutes: 2)), ); + test( - 'Check health workflow "${check.name}" against golden files', + 'Check health workflow "${check.name}" against golden files ' + 'with health.yaml changed itself', () async => await checkGolden( check, fakeGithubApi([ From eb1935690cec8961f7784b5c4b98e7b5ae9020bd Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 18:29:10 +0200 Subject: [PATCH 22/32] Add changelog --- pkgs/firehose/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index 6890c0d0..ba51b25b 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -2,6 +2,8 @@ - Remove the `version` pubspec checks (these largely duplicate the feedback provided by publishing automation). +- Run health workflow on all packages if it is changed. +- Specify Flutter packages in the repo, to only have a single workflow file. ## 0.9.3 From b7610e00fa0712798ad6c6601dc8b99482b6b489 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 18:36:20 +0200 Subject: [PATCH 23/32] Compare to last published version in breaking check. --- pkgs/firehose/CHANGELOG.md | 1 + pkgs/firehose/lib/src/health/health.dart | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index ba51b25b..2048bd2c 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -4,6 +4,7 @@ provided by publishing automation). - Run health workflow on all packages if it is changed. - Specify Flutter packages in the repo, to only have a single workflow file. +- Compare to last published version in breaking check. ## 0.9.3 diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 14440c0e..e6aecee2 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -149,8 +149,9 @@ class Health { ...['pub', 'global', 'run'], 'dart_apitool:main', 'diff', + '--no-check-sdk-version', if (flutter.contains(package)) '--force-use-flutter', - ...['--old', baseRelativePath], + ...['--old', 'pub://${package.name}'], ...['--new', relativePath], ...['--report-format', 'json'], ...['--report-file-path', reportPath], From e0273ea3a477b6da371686123d5f58a0016de8b3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 24 Oct 2024 18:45:29 +0200 Subject: [PATCH 24/32] Fix bug --- pkgs/firehose/lib/src/health/health.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index e6aecee2..3b8b106f 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -138,9 +138,6 @@ class Health { log('Look for changes in $package with base $baseDirectory'); var relativePath = path.relative(package.directory.path, from: directory.path); - var baseRelativePath = path.relative( - path.join(baseDirectory.path, relativePath), - from: directory.path); var tempDirectory = Directory.systemTemp.createTempSync(); var reportPath = path.join(tempDirectory.path, 'report.json'); var runApiTool = Process.runSync( @@ -397,7 +394,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ); var files = await listFilesInPRorAll(ignoredPackages); - var coverageResult = coverage.compareCoveragesFor(files, directory); + var coverageResult = coverage.compareCoveragesFor(files, baseDirectory); var markdownResult = ''' | File | Coverage | From d02958157ad08305a36a8894da3fd34ba7af9c0b Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 31 Oct 2024 14:34:02 +0100 Subject: [PATCH 25/32] Update .github/workflows/health_base.yaml Co-authored-by: Devon Carew --- .github/workflows/health_base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index d3eaef42..1e937bf0 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -18,7 +18,7 @@ on: channel: description: >- The channel, or a specific version from a channel, to install - ('2.19.0','stable', 'beta', 'dev'). Using one of the three channels + ('2.19.0','stable', 'beta', 'dev'). Using one of the three named channels will give you the latest version published to that channel. default: "stable" required: false From d227021abb0aa60fe6168a6b6fad5feb6ad88000 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 14 Nov 2024 14:17:09 +0100 Subject: [PATCH 26/32] Echo dart versions --- .github/workflows/health_base.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 255d6c99..8b681ad0 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -116,6 +116,12 @@ jobs: with: channel: ${{ inputs.channel }} + - name: Check Dart installs whereis + run: whereis dart + + - name: Check Dart installs which + run: which dart + - name: Install coverage run: dart pub global activate coverage if: ${{ inputs.check == 'coverage' }} From 1aff2750c045a0be321a8e395c702cdb85f0d550 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 14 Nov 2024 19:21:16 +0100 Subject: [PATCH 27/32] Use different dart and flutter binaries --- pkgs/firehose/lib/src/health/coverage.dart | 10 +++--- pkgs/firehose/lib/src/health/health.dart | 37 +++++++++++++++++----- pkgs/firehose/test/coverage_test.dart | 2 +- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index cb08daba..c4d76781 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -20,6 +20,7 @@ class Coverage { final List ignoredPackages; final Directory directory; final List experiments; + final String dartExecutable; Coverage( this.coverageWeb, @@ -27,6 +28,7 @@ class Coverage { this.ignoredPackages, this.directory, this.experiments, + this.dartExecutable, ); CoverageResult compareCoveragesFor(List files, Directory base) { @@ -99,7 +101,7 @@ class Coverage { print(''' Get coverage for ${package.name} by running coverage in ${package.directory.path}'''); Process.runSync( - 'dart', + dartExecutable, [ if (experiments.isNotEmpty) '--enable-experiment=${experiments.join(',')}', @@ -111,7 +113,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path if (coverageWeb) { print('Run tests with coverage for web'); var resultChrome = Process.runSync( - 'dart', + dartExecutable, [ if (experiments.isNotEmpty) '--enable-experiment=${experiments.join(',')}', @@ -130,7 +132,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path print('Run tests with coverage for vm'); var resultVm = Process.runSync( - 'dart', + dartExecutable, [ if (experiments.isNotEmpty) '--enable-experiment=${experiments.join(',')}', @@ -146,7 +148,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path print('Compute coverage from runs'); var resultLcov = Process.runSync( - 'dart', + dartExecutable, [ 'pub', 'global', diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 3b8b106f..b1186605 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -53,7 +53,7 @@ class Health { String? comment, this.log = printLogger, }) : ignoredPackages = toGlobs(ignoredPackages), - flutterPackages = toGlobs(flutterPackages), + flutterPackageGlobs = toGlobs(flutterPackages), ignoredFilesForCoverage = toGlobs(ignoredCoverage), ignoredFilesForLicense = toGlobs(ignoredLicense), baseDirectory = base ?? Directory('../base_repo'), @@ -62,7 +62,16 @@ class Health { directory.path, 'output', 'comment.md', - ); + ) { + var split = (Process.runSync('whereis', ['-a', 'dart']).stdout as String) + .split('\n'); + + flutterExecutable = + split.firstWhereOrNull((path) => path.contains('flutter')); + dartExecutable = + split.firstWhereOrNull((path) => !path.contains('flutter')) ?? + flutterExecutable!; + } static List toGlobs(List ignoredPackages) => ignoredPackages.map((pattern) => Glob(pattern, recursive: true)).toList(); @@ -76,11 +85,17 @@ class Health { final List ignoredPackages; final List ignoredFilesForLicense; final List ignoredFilesForCoverage; - final List flutterPackages; + final List flutterPackageGlobs; final Directory baseDirectory; final List experiments; final Logger log; + late final String dartExecutable; + late final String? flutterExecutable; + + String executable(bool isFlutter) => + isFlutter ? flutterExecutable ?? dartExecutable : dartExecutable; + Future healthCheck() async { // Do basic validation of our expected env var. if (!expectEnv(github.repoSlug?.fullName, 'GITHUB_REPOSITORY')) return; @@ -92,7 +107,7 @@ class Health { log(' warnOn: $warnOn'); log(' failOn: $failOn'); log(' coverageweb: $coverageweb'); - log(' flutterPackages: $flutterPackages'); + log(' flutterPackages: $flutterPackageGlobs'); log(' ignoredPackages: $ignoredPackages'); log(' ignoredForLicense: $ignoredFilesForLicense'); log(' ignoredForCoverage: $ignoredFilesForCoverage'); @@ -131,7 +146,8 @@ class Health { Future breakingCheck() async { final filesInPR = await listFilesInPRorAll(ignoredPackages); final changeForPackage = {}; - final flutter = packagesContaining(filesInPR, only: flutterPackages); + final flutterPackages = + packagesContaining(filesInPR, only: flutterPackageGlobs); for (var package in packagesContaining(filesInPR, ignore: ignoredPackages)) { @@ -140,14 +156,15 @@ class Health { path.relative(package.directory.path, from: directory.path); var tempDirectory = Directory.systemTemp.createTempSync(); var reportPath = path.join(tempDirectory.path, 'report.json'); + var isFlutterPackage = flutterPackages.contains(package); var runApiTool = Process.runSync( - 'dart', + executable(isFlutterPackage), [ ...['pub', 'global', 'run'], 'dart_apitool:main', 'diff', '--no-check-sdk-version', - if (flutter.contains(package)) '--force-use-flutter', + if (isFlutterPackage) '--force-use-flutter', ...['--old', 'pub://${package.name}'], ...['--new', relativePath], ...['--report-format', 'json'], @@ -205,6 +222,9 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} Future leakingCheck() async { var filesInPR = await listFilesInPRorAll(ignoredPackages); final leaksForPackage = >{}; + + final flutterPackages = + packagesContaining(filesInPR, only: flutterPackageGlobs); for (var package in packagesContaining(filesInPR)) { log('Look for leaks in $package'); var relativePath = @@ -219,7 +239,7 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} ...['--output', reportPath], ]; var runApiTool = Process.runSync( - 'dart', + executable(flutterPackages.contains(package)), arguments, workingDirectory: directory.path, ); @@ -391,6 +411,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ignoredPackages, directory, experiments, + dartExecutable, ); var files = await listFilesInPRorAll(ignoredPackages); diff --git a/pkgs/firehose/test/coverage_test.dart b/pkgs/firehose/test/coverage_test.dart index 9a066635..876a0a8d 100644 --- a/pkgs/firehose/test/coverage_test.dart +++ b/pkgs/firehose/test/coverage_test.dart @@ -43,7 +43,7 @@ void main() { } class FakeHealth extends Coverage { - FakeHealth() : super(true, [], [], Directory.current, []); + FakeHealth() : super(true, [], [], Directory.current, [], ''); @override Map getCoverage(Package? package) { From 69709691093534018addf6712e6f47b661d2204e Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 14 Nov 2024 19:24:48 +0100 Subject: [PATCH 28/32] Add command --- pkgs/firehose/test/coverage_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/firehose/test/coverage_test.dart b/pkgs/firehose/test/coverage_test.dart index 876a0a8d..63abdbcf 100644 --- a/pkgs/firehose/test/coverage_test.dart +++ b/pkgs/firehose/test/coverage_test.dart @@ -43,7 +43,7 @@ void main() { } class FakeHealth extends Coverage { - FakeHealth() : super(true, [], [], Directory.current, [], ''); + FakeHealth() : super(true, [], [], Directory.current, [], 'dart'); @override Map getCoverage(Package? package) { From 3a47a4eb8d3cf2f4c9bccea9b250c8a0c16f3508 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 14 Nov 2024 19:27:36 +0100 Subject: [PATCH 29/32] Fixes --- pkgs/firehose/lib/src/health/health.dart | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index b1186605..d145ccf7 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -63,14 +63,17 @@ class Health { 'output', 'comment.md', ) { - var split = (Process.runSync('whereis', ['-a', 'dart']).stdout as String) - .split('\n'); - flutterExecutable = - split.firstWhereOrNull((path) => path.contains('flutter')); - dartExecutable = - split.firstWhereOrNull((path) => !path.contains('flutter')) ?? - flutterExecutable!; + (Process.runSync('whereis', ['-a', 'dart']).stdout as String) + .split('\n') + .firstOrNull; + + var dartExecutables = + (Process.runSync('whereis', ['-a', 'dart']).stdout as String) + .split('\n'); + dartExecutable = dartExecutables + .firstWhereOrNull((path) => !path.contains('flutter')) ?? + dartExecutables.firstWhereOrNull((path) => path.contains('flutter'))!; } static List toGlobs(List ignoredPackages) => @@ -164,7 +167,6 @@ class Health { 'dart_apitool:main', 'diff', '--no-check-sdk-version', - if (isFlutterPackage) '--force-use-flutter', ...['--old', 'pub://${package.name}'], ...['--new', relativePath], ...['--report-format', 'json'], From 9035f648bf0f41e56eecb6eea4d10f9bfa90f91d Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 14 Nov 2024 19:52:52 +0100 Subject: [PATCH 30/32] Fix --- pkgs/firehose/lib/src/health/health.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index d145ccf7..0f909b19 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -64,16 +64,18 @@ class Health { 'comment.md', ) { flutterExecutable = - (Process.runSync('whereis', ['-a', 'dart']).stdout as String) + (Process.runSync('which', ['-a', 'flutter']).stdout as String) .split('\n') + .where((element) => element.isNotEmpty) .firstOrNull; var dartExecutables = - (Process.runSync('whereis', ['-a', 'dart']).stdout as String) - .split('\n'); + (Process.runSync('which', ['-a', 'dart']).stdout as String) + .split('\n') + .where((element) => element.isNotEmpty); dartExecutable = dartExecutables - .firstWhereOrNull((path) => !path.contains('flutter')) ?? - dartExecutables.firstWhereOrNull((path) => path.contains('flutter'))!; + .sortedBy((path) => path.contains('flutter').toString()) + .first; } static List toGlobs(List ignoredPackages) => From 31aaf3fff5c36f712cf9bae4f4f1e5722fb8cc45 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Dec 2024 15:16:54 +0100 Subject: [PATCH 31/32] Fix ref error --- pkgs/firehose/lib/src/health/health.dart | 60 +++++++++++++++--------- pkgs/firehose/test/health_test.dart | 26 +++++++++- 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 0f909b19..18dd388f 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -156,28 +156,25 @@ class Health { for (var package in packagesContaining(filesInPR, ignore: ignoredPackages)) { - log('Look for changes in $package with base $baseDirectory'); + log('Look for changes in $package'); var relativePath = path.relative(package.directory.path, from: directory.path); var tempDirectory = Directory.systemTemp.createTempSync(); var reportPath = path.join(tempDirectory.path, 'report.json'); - var isFlutterPackage = flutterPackages.contains(package); - var runApiTool = Process.runSync( - executable(isFlutterPackage), + runDashProcess( + flutterPackages, + package, [ ...['pub', 'global', 'run'], 'dart_apitool:main', 'diff', '--no-check-sdk-version', - ...['--old', 'pub://${package.name}'], + ...['--old', getCurrentVersionOfPackage(package)], ...['--new', relativePath], ...['--report-format', 'json'], ...['--report-file-path', reportPath], ], - workingDirectory: directory.path, ); - log(runApiTool.stderr as String); - log(runApiTool.stdout as String); var fullReportString = File(reportPath).readAsStringSync(); var decoded = jsonDecode(fullReportString) as Map; @@ -209,6 +206,22 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} ); } + String getCurrentVersionOfPackage(Package package) => 'pub://${package.name}'; + + ProcessResult runDashProcess( + List flutterPackages, Package package, List arguments) { + var exec = executable(flutterPackages.contains(package)); + log('Running `$exec ${arguments.join(' ')}` in ${directory.path}'); + var runApiTool = Process.runSync( + exec, + arguments, + workingDirectory: directory.path, + ); + log(runApiTool.stderr as String); + log(runApiTool.stdout as String); + return runApiTool; + } + BreakingLevel _breakingLevel(Map report) { BreakingLevel breakingLevel; if ((report['noChangesDetected'] as bool?) ?? false) { @@ -235,20 +248,17 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} path.relative(package.directory.path, from: directory.path); var tempDirectory = Directory.systemTemp.createTempSync(); var reportPath = path.join(tempDirectory.path, 'leaks.json'); - var arguments = [ - ...['pub', 'global', 'run'], - 'dart_apitool:main', - 'extract', - ...['--input', relativePath], - ...['--output', reportPath], - ]; - var runApiTool = Process.runSync( - executable(flutterPackages.contains(package)), - arguments, - workingDirectory: directory.path, + var runApiTool = runDashProcess( + flutterPackages, + package, + [ + ...['pub', 'global', 'run'], + 'dart_apitool:main', + 'extract', + ...['--input', relativePath], + ...['--output', reportPath], + ], ); - log(runApiTool.stderr as String); - log(runApiTool.stdout as String); if (runApiTool.exitCode == 0) { var fullReportString = await File(reportPath).readAsString(); @@ -262,7 +272,13 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} } else { throw ProcessException( 'Api tool finished with exit code ${runApiTool.exitCode}', - arguments, + [ + ...['pub', 'global', 'run'], + 'dart_apitool:main', + 'extract', + ...['--input', relativePath], + ...['--output', reportPath], + ], ); } } diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart index 0c830cd5..d17e6db2 100644 --- a/pkgs/firehose/test/health_test.dart +++ b/pkgs/firehose/test/health_test.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:firehose/src/github.dart'; import 'package:firehose/src/health/health.dart'; +import 'package:firehose/src/repo.dart'; import 'package:github/src/common/model/repos.dart'; import 'package:glob/glob.dart'; import 'package:path/path.dart' as p; @@ -119,7 +120,7 @@ Future checkGolden( }) async { final commentPath = p.join( Directory.systemTemp.createTempSync().path, 'comment_${check.name}.md'); - await Health( + await FakeHealth( directory, check, [], @@ -145,6 +146,29 @@ Future checkGolden( } } +class FakeHealth extends Health { + FakeHealth( + super.directory, + super.check, + super.warnOn, + super.failOn, + super.coverageweb, + super.ignoredPackages, + super.ignoredLicense, + super.ignoredCoverage, + super.experiments, + super.github, + super.flutterPackages, { + super.base, + super.comment, + super.log, + }); + + @override + String getCurrentVersionOfPackage(Package package) => + p.join('../base_test_repo/pkgs', package.name); +} + class FakeGithubApi implements GithubApi { final List files; From f1d36800e2886940506bb37073bfdafdbd510e83 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 6 Dec 2024 13:34:02 +0100 Subject: [PATCH 32/32] Fix to covered --- .../firehose/test_data/golden/comment_coverage_healthchanged.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md b/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md index 6141d8d2..5de5ebe3 100644 --- a/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md +++ b/pkgs/firehose/test_data/golden/comment_coverage_healthchanged.md @@ -11,7 +11,7 @@ |pkgs/package2/lib/anotherLib.dart| :green_heart: 100 % | |pkgs/package2/lib/package2.dart| :green_heart: 100 % | |pkgs/package3/bin/package3.dart| :broken_heart: Not covered | -|pkgs/package3/lib/package3.dart| :broken_heart: Not covered | +|pkgs/package3/lib/package3.dart| :green_heart: 100 % | |pkgs/package5/lib/package5.dart| :broken_heart: Not covered | |pkgs/package5/lib/src/package5_base.dart| :broken_heart: Not covered |