Skip to content

Commit

Permalink
Enable experiments for health (#226)
Browse files Browse the repository at this point in the history
* Enable `enable-experiment` option for health

* Fix DNS bug

* Add default

* Add default to yaml

* Add default to other wf

* Add debug message

* Remove empty from list

* Add empty arg

* Rev version

* join by comma

* Fix typos
  • Loading branch information
mosuem authored Jan 16, 2024
1 parent c81f25c commit f61a550
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 19 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ on:
default: false
required: false
type: boolean
experiments:
description: Which experiments should be enabled for Dart.
default: "\"\""
type: string
required: false

jobs:
version:
Expand All @@ -101,6 +106,7 @@ jobs:
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
checkout_submodules: ${{ inputs.checkout_submodules }}

changelog:
if: ${{ contains(inputs.checks, 'changelog') }}
uses: ./.github/workflows/health_base.yaml
Expand Down Expand Up @@ -138,6 +144,7 @@ jobs:
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
checkout_submodules: ${{ inputs.checkout_submodules }}
experiments: ${{ inputs.experiments }}

breaking:
if: ${{ contains(inputs.checks, 'breaking') }}
Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ on:
default: false
required: false
type: boolean
experiments:
description: Which experiments should be enabled for Dart.
default: "\"\""
type: string
required: false

jobs:
health:
Expand Down Expand Up @@ -119,7 +124,8 @@ jobs:
--check ${{ inputs.check }} \
${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} \
--fail_on ${{ inputs.fail_on }} \
--warn_on ${{ inputs.warn_on }}
--warn_on ${{ inputs.warn_on }} \
--experiments ${{ inputs.experiments }}
- run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.check }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md
if: ${{ '$action_state' == 1 }}
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.5.3

- Allow experiments to be enabled for Dart.

## 0.5.2

- Also run health workflows on bot PRs.
Expand Down
17 changes: 15 additions & 2 deletions pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ void main(List<String> arguments) async {
allowed: checkTypes,
help: 'Which checks should lead to workflow failure',
)
..addMultiOption(
'experiments',
help: 'Which experiments should be enabled for Dart',
)
..addFlag(
'coverage_web',
help: 'Whether to run web tests for coverage',
Expand All @@ -32,11 +36,20 @@ void main(List<String> arguments) async {
var check = parsedArgs['check'] as String;
var warnOn = parsedArgs['warn_on'] as List<String>;
var failOn = parsedArgs['fail_on'] as List<String>;
var experiments = (parsedArgs['experiments'] as List<String>)
.where((name) => name.isNotEmpty)
.toList();
var coverageWeb = parsedArgs['coverage_web'] as bool;
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.');
}
await Health(Directory.current, check, warnOn, failOn, coverageWeb)
.healthCheck();
await Health(
Directory.current,
check,
warnOn,
failOn,
coverageWeb,
experiments,
).healthCheck();
}
26 changes: 22 additions & 4 deletions pkgs/firehose/lib/src/health/coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import 'lcov.dart';

class Coverage {
final bool coverageWeb;
final List<String> experiments;

Coverage(this.coverageWeb);
Coverage(this.coverageWeb, this.experiments);

Future<CoverageResult> compareCoverages(GithubApi github) async {
var files = await github.listFilesForPR();
Expand Down Expand Up @@ -93,14 +94,26 @@ class Coverage {
Get coverage for ${package.name} by running coverage in ${package.directory.path}''');
Process.runSync(
'dart',
['pub', 'get'],
[
if (experiments.isNotEmpty)
'--enable-experiment=${experiments.join(',')}',
'pub',
'get'
],
workingDirectory: package.directory.path,
);
if (coverageWeb) {
print('Get test coverage for web');
var resultChrome = Process.runSync(
'dart',
['test', '-p', 'chrome', '--coverage=coverage'],
[
if (experiments.isNotEmpty)
'--enable-experiment=${experiments.join(',')}',
'test',
'-p',
'chrome',
'--coverage=coverage'
],
workingDirectory: package.directory.path,
);
print(resultChrome.stdout);
Expand All @@ -109,7 +122,12 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path
print('Get test coverage for vm');
var resultVm = Process.runSync(
'dart',
['test', '--coverage=coverage'],
[
if (experiments.isNotEmpty)
'--enable-experiment=${experiments.join(',')}',
'test',
'--coverage=coverage'
],
workingDirectory: package.directory.path,
);
print(resultVm.stdout);
Expand Down
25 changes: 15 additions & 10 deletions pkgs/firehose/lib/src/health/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ class Health {
this.warnOn,
this.failOn,
this.coverageweb,
this.experiments,
);
final github = GithubApi();

final String check;
final List<String> warnOn;
final List<String> failOn;
final bool coverageweb;
final List<String> experiments;

Future<void> healthCheck() async {
// Do basic validation of our expected env var.
Expand Down Expand Up @@ -259,24 +261,26 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst
}

Future<HealthCheckResult> doNotSubmitCheck() async {
final dns = 'DO_NOT${'_'}SUBMIT';

final body = await github.pullrequestBody();
final files = await github.listFilesForPR();
print('Checking for DO_NOT${'_'}SUBMIT strings: $files');
print('Checking for $dns strings: $files');
final filesWithDNS = files
.where((file) =>
![FileStatus.removed, FileStatus.unchanged].contains(file.status))
.where((file) => File(file.relativePath)
.readAsStringSync()
.contains('DO_NOT${'_'}SUBMIT'))
.where((file) => File(file.relativePath).existsSync())
.where(
(file) => File(file.relativePath).readAsStringSync().contains(dns))
.toList();
print('Found files with DO_NOT_${'SUBMIT'}: $filesWithDNS');
print('Found files with $dns: $filesWithDNS');

final bodyContainsDNS = body.contains('DO_NOT${'_'}SUBMIT');
print('The body contains a DO_NOT${'_'}SUBMIT string: $bodyContainsDNS');
final bodyContainsDNS = body.contains(dns);
print('The body contains a $dns string: $bodyContainsDNS');
final markdownResult = '''
Body contains `DO_NOT${'_'}SUBMIT`: $bodyContainsDNS
Body contains `$dns`: $bodyContainsDNS
| Files with `DO_NOT_${'SUBMIT'}` |
| Files with `$dns` |
| :--- |
${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}
''';
Expand All @@ -290,7 +294,8 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}
}

Future<HealthCheckResult> coverageCheck() async {
var coverage = await Coverage(coverageweb).compareCoverages(github);
var coverage =
await Coverage(coverageweb, experiments).compareCoverages(github);

var markdownResult = '''
| File | Coverage |
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: firehose
description: A tool to automate publishing of Pub packages from GitHub actions.
version: 0.5.2
version: 0.5.3
repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose

environment:
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/test/coverage_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void main() {
}

class FakeHealth extends Coverage {
FakeHealth() : super(true);
FakeHealth() : super(true, []);

@override
Map<String, double> getCoverage(Package? package) {
Expand Down

0 comments on commit f61a550

Please sign in to comment.