Skip to content

Commit

Permalink
Updates to health.yaml (#310)
Browse files Browse the repository at this point in the history
- 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.

This is a breaking change, as the `use-flutter` argument is not existing
anymore. After landing this, all `dart-lang/` repos will have to be
updated. As part of that update, we should maybe have the `health.yaml`s
depend on a hash instead of the `main` branch.

---

- [x] I’ve reviewed the contributor guide and applied the relevant
portions to this PR.

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor
guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md)
for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before
creating a PR.
- Contributions to our repos should follow the [Dart style
guide](https://dart.dev/guides/language/effective-dart) and use `dart
format`.
- Most changes should add an entry to the changelog and may need to [rev
the pubspec package
version](https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change).
- Changes to packages require [corresponding
tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs -
please allow for some latency before initial review feedback.
</details>

---------

Co-authored-by: Devon Carew <[email protected]>
  • Loading branch information
mosuem and devoncarew authored Dec 6, 2024
1 parent 10fb2c0 commit 8749a2b
Show file tree
Hide file tree
Showing 17 changed files with 401 additions and 123 deletions.
28 changes: 17 additions & 11 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ name: Health
# warn_on: "license,coverage,breaking,leaking"
# coverage_web: false
# upload_coverage: false
# use-flutter: true
# flutter_packages: "pkgs/my_flutter_package"
# ignore_license: "**.g.dart"
# ignore_coverage: "**.mock.dart,**.g.dart"
# ignore_packages: "pkgs/helper_package"
Expand All @@ -42,6 +42,12 @@ on:
# parameter is not required; it defaults to `stable` - using the most recent
# stable release of the Dart SDK.
sdk:
description: >-
The Dart SDK version, either a semver or one of `dev`, `stable` etc.
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
Expand Down Expand Up @@ -81,11 +87,11 @@ on:
default: false
type: boolean
required: false
use-flutter:
description: Whether to setup Flutter in this workflow.
default: false
flutter_packages:
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: "\"\""
Expand Down Expand Up @@ -122,7 +128,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 }}

Expand All @@ -135,7 +141,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 }}
Expand All @@ -151,7 +157,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 }}
Expand All @@ -166,7 +172,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 }}

Expand All @@ -179,7 +185,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 }}

Expand All @@ -192,7 +198,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 }}

Expand Down
37 changes: 26 additions & 11 deletions .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 named 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
Expand Down Expand Up @@ -44,11 +52,11 @@ on:
default: false
type: boolean
required: false
use-flutter:
description: Whether to setup Flutter in this workflow.
default: false
flutter_packages:
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: "\"\""
Expand Down Expand Up @@ -98,23 +106,29 @@ jobs:
if: ${{ inputs.check == 'coverage' }} || ${{ inputs.check == 'breaking' }}

- run: mkdir -p current_repo/output/

- uses: subosito/flutter-action@74af56c5ed2697ba4621264652728e8d217e53d3
if: ${{ inputs.use-flutter }}
with:
channel: ${{ inputs.sdk }}

- uses: dart-lang/setup-dart@e630b99d28a3b71860378cafdc2a067c71107f94
if: ${{ !inputs.use-flutter }}
with:
sdk: ${{ inputs.sdk }}

- uses: subosito/flutter-action@74af56c5ed2697ba4621264652728e8d217e53d3
if: ${{ inputs.flutter_packages != '' }}
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' }}

- 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
# DO-NOT-SUBMIT
if: ${{ !inputs.local_debug }}

- name: Install local firehose
Expand All @@ -139,6 +153,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 }} \
Expand Down
3 changes: 3 additions & 0 deletions pkgs/firehose/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
- Remove the `version` pubspec checks (these largely duplicate the feedback
provided by publishing automation).
- Set minimum SDK version to `3.5.0` because of the `dart_apitool` dependency.
- 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

Expand Down
15 changes: 11 additions & 4 deletions pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,23 @@ void main(List<String> 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 checkStr = parsedArgs.option('check');
final check = Check.values.firstWhere((c) => c.name == checkStr);
final warnOn = parsedArgs['warn_on'] as List<String>;
final failOn = parsedArgs['fail_on'] as List<String>;
final warnOn = parsedArgs.multiOption('warn_on');
final failOn = parsedArgs.multiOption('fail_on');
final flutterPackages = _listNonEmpty(parsedArgs, '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.');
Expand All @@ -74,6 +80,7 @@ void main(List<String> arguments) async {
ignoreCoverage,
experiments,
GithubApi(),
flutterPackages,
).healthCheck();
}

Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/lib/firehose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');

Future<VerificationResults> verify(GithubApi github) async {
var repo = Repository(directory);
var packages = repo.locatePackages(ignoredPackages);
var packages = repo.locatePackages(ignore: ignoredPackages);

var pub = Pub();

Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/lib/src/health/changelog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Future<Map<Package, List<GitFile>>> 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);

Expand Down
20 changes: 8 additions & 12 deletions pkgs/firehose/lib/src/health/coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,20 @@ class Coverage {
final List<Glob> ignoredPackages;
final Directory directory;
final List<String> experiments;
final String dartExecutable;

Coverage(
this.coverageWeb,
this.ignoredFiles,
this.ignoredPackages,
this.directory,
this.experiments,
this.dartExecutable,
);

Future<CoverageResult> compareCoverages(
GithubApi github, Directory base) async {
var files = await github.listFilesForPR(directory, ignoredFiles);
return compareCoveragesFor(files, base);
}

CoverageResult compareCoveragesFor(List<GitFile> 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
Expand All @@ -49,7 +45,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
Expand Down Expand Up @@ -105,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(',')}',
Expand All @@ -117,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(',')}',
Expand All @@ -136,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(',')}',
Expand All @@ -152,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',
Expand Down
Loading

0 comments on commit 8749a2b

Please sign in to comment.