Skip to content

Commit

Permalink
Add check for API leaks to the Health workflow (#251)
Browse files Browse the repository at this point in the history
* Add checks for API leaks

* Fixes

* More fixes

* Fix SDK version in testdata

* Install dart_apitool on leaking in action

* Update tests

* Fix message

* Add exports

* Fix analyze issues

* Reset tests after adding a period
  • Loading branch information
mosuem committed Apr 15, 2024
1 parent 95fed18 commit 9fabe46
Show file tree
Hide file tree
Showing 32 changed files with 284 additions and 101 deletions.
29 changes: 21 additions & 8 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ name: Health
# uses: dart-lang/ecosystem/.github/workflows/health.yaml@main
# with:
# sdk: beta
# checks: "version,changelog,license,coverage,breaking,do-not-submit"
# checks: "version,changelog,license,coverage,breaking,do-not-submit,leaking"
# fail_on: "version,changelog,do-not-submit"
# warn_on: "license,coverage,breaking"
# warn_on: "license,coverage,breaking,leaking"
# coverage_web: false
# upload_coverage: false
# use-flutter: true
Expand Down Expand Up @@ -54,18 +54,18 @@ on:
# Restrict the checks to any subset of version, changelog, and license if
# needed.
checks:
description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
default: "version,changelog,license,coverage,breaking,do-not-submit"
description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
default: "version,changelog,license,coverage,breaking,do-not-submit,leaking"
type: string
required: false
fail_on:
description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
default: "version,changelog,do-not-submit"
type: string
required: false
warn_on:
description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
default: "license,coverage,breaking"
description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
default: "license,coverage,breaking,leaking"
type: string
required: false
local_debug:
Expand Down Expand Up @@ -198,8 +198,21 @@ jobs:
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

leaking:
if: ${{ contains(inputs.checks, 'leaking') }}
uses: ./.github/workflows/health_base.yaml
with:
sdk: ${{ inputs.sdk }}
check: leaking
fail_on: ${{ inputs.fail_on }}
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

comment:
needs: [version, changelog, license, coverage, breaking, do-not-submit]
needs: [version, changelog, license, coverage, breaking, do-not-submit, leaking]
if: always()
# These permissions are required for us to create comments on PRs.
permissions:
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ on:
required: false
type: string
check:
description: What to check for in the PR health check - any of "version,changelog,license,coverage,breaking,do-not-submit"
description: What to check for in the PR health check - any of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
type: string
required: true
fail_on:
description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
default: "version,changelog,do-not-submit"
type: string
required: false
warn_on:
description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
default: "license,coverage,breaking"
description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit,leaking"
default: "license,coverage,breaking,leaking"
type: string
required: false
local_debug:
Expand Down Expand Up @@ -123,7 +123,7 @@ jobs:

- name: Install api_tool
run: dart pub global activate dart_apitool
if: ${{ inputs.check == 'breaking' }}
if: ${{ inputs.check == 'breaking' || inputs.check == 'leaking' }}

- name: Check PR health
id: healthstep
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/health_internal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ jobs:
local_debug: true
coverage_web: false
upload_coverage: false
checks: version,changelog,license,coverage,breaking,do-not-submit
checks: version,changelog,license,coverage,breaking,do-not-submit,leaking
fail_on: version,changelog,do-not-submit
warn_on: license,coverage,breaking
warn_on: license,coverage,breaking,leaking
ignore_license: 'pkgs/firehose/test_data'
ignore_coverage: 'pkgs/firehose/bin'
ignore_coverage: 'pkgs/firehose/bin,pkgs/firehose/test_data'
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
},
"args": [
"--checks",
"version,changelog,license,coverage,do-not-submit"
"version,changelog,license,coverage,do-not-submit,leaking"
]
}
]
Expand Down
4 changes: 4 additions & 0 deletions pkgs/firehose/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.0

- Add `leaking` check to the health workflow.

## 0.8.0

- Only check text files for do not submit strings.
Expand Down
5 changes: 3 additions & 2 deletions pkgs/firehose/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ When run from a PR, this tool will check a configurable subset of the following
* How the test coverage is affected by the PR.
* The package versioning takes into account any breaking changes in the PR.
* The PR contains `DO_NOT_SUBMIT` strings in the files or the description.
* Any symbols are visible in the public API, but not exported.

This tool can work with either single package repos or with mono-repos (repos
containing several packages).
Expand All @@ -192,9 +193,9 @@ jobs:

| Name | Type | Description | Example |
| ------------- | ------------- | ------------- | ------------- |
| checks | List of strings | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit"` |
| checks | List of strings | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit,leaking"` |
| fail_on | List of strings | Which checks should lead to failure | `"version,changelog,do-not-submit"` |
| warn_on | List of strings | Which checks should not fail, but only warn | `"license,coverage,breaking"` |
| warn_on | List of strings | Which checks should not fail, but only warn | `"license,coverage,breaking,leaking"` |
| upload_coverage | boolean | Whether to upload the coverage to [coveralls](https://coveralls.io/) | `true` |
| coverage_web | boolean | Whether to run `dart test -p chrome` for coverage | `false` |
| use-flutter | boolean | Whether to setup Flutter in this workflow | `false` |
Expand Down
1 change: 0 additions & 1 deletion pkgs/firehose/bin/firehose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:io';

import 'package:args/args.dart';
import 'package:firehose/firehose.dart';
import 'package:firehose/src/github.dart';
import 'package:glob/glob.dart';

const helpFlag = 'help';
Expand Down
4 changes: 3 additions & 1 deletion pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:firehose/src/github.dart';
import 'package:firehose/src/health/health.dart';

void main(List<String> arguments) async {
var checkTypes = Check.values.map((c) => c.name);
var argParser = ArgParser()
..addOption(
'check',
Expand Down Expand Up @@ -49,7 +50,8 @@ void main(List<String> arguments) async {
help: 'Whether to run web tests for coverage',
);
final parsedArgs = argParser.parse(arguments);
final check = parsedArgs['check'] as String;
final checkStr = parsedArgs['check'] as String;
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 ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages');
Expand Down
5 changes: 5 additions & 0 deletions pkgs/firehose/lib/firehose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import 'src/pub.dart';
import 'src/repo.dart';
import 'src/utils.dart';

export 'src/changelog.dart' show Changelog;
export 'src/github.dart' show FileStatus, GitFile, GithubApi;
export 'src/repo.dart' show Package, Repository;
export 'src/utils.dart' show Severity;

const String _botSuffix = '[bot]';

const String _githubActionsUser = 'github-actions[bot]';
Expand Down
Loading

0 comments on commit 9fabe46

Please sign in to comment.