From afa9da9b71af41c273a4ca6dbc39c50ebb0c0709 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 10:58:43 +0100 Subject: [PATCH 01/11] Enable `enable-experiment` option for health --- .github/workflows/health.yaml | 7 +++++++ .github/workflows/health_base.yaml | 8 +++++++- pkgs/firehose/bin/health.dart | 15 ++++++++++++-- pkgs/firehose/lib/src/health/coverage.dart | 23 ++++++++++++++++++---- pkgs/firehose/lib/src/health/health.dart | 5 ++++- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 46cad959..2d07db6c 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -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: @@ -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 @@ -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') }} diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 92a451a6..c9ab800a 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -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: @@ -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 }} diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 19a98d40..cbf6bd21 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -24,6 +24,10 @@ void main(List 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', @@ -32,11 +36,18 @@ void main(List arguments) async { var check = parsedArgs['check'] as String; var warnOn = parsedArgs['warn_on'] as List; var failOn = parsedArgs['fail_on'] as List; + var experiments = parsedArgs['experiments'] as List; 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(); } diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index b1e8aa12..b2513180 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -14,8 +14,9 @@ import 'lcov.dart'; class Coverage { final bool coverageWeb; + final List experiments; - Coverage(this.coverageWeb); + Coverage(this.coverageWeb, this.experiments); Future compareCoverages(GithubApi github) async { var files = await github.listFilesForPR(); @@ -93,14 +94,24 @@ 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', + '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', + 'test', + '-p', + 'chrome', + '--coverage=coverage' + ], workingDirectory: package.directory.path, ); print(resultChrome.stdout); @@ -109,7 +120,11 @@ 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', + 'test', + '--coverage=coverage' + ], workingDirectory: package.directory.path, ); print(resultVm.stdout); diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 7f1ce1e7..a93cd93d 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -50,6 +50,7 @@ class Health { this.warnOn, this.failOn, this.coverageweb, + this.experiments, ); final github = GithubApi(); @@ -57,6 +58,7 @@ class Health { final List warnOn; final List failOn; final bool coverageweb; + final List experiments; Future healthCheck() async { // Do basic validation of our expected env var. @@ -290,7 +292,8 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} } Future coverageCheck() async { - var coverage = await Coverage(coverageweb).compareCoverages(github); + var coverage = + await Coverage(coverageweb, experiments).compareCoverages(github); var markdownResult = ''' | File | Coverage | From 3a8317da18a1364fe360b7cda52f3f82e19d7e14 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:03:06 +0100 Subject: [PATCH 02/11] Fix DNS bug --- pkgs/firehose/lib/src/health/health.dart | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index a93cd93d..5acd7e51 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -261,24 +261,26 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst } Future 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')) - .toList(); - print('Found files with DO_NOT_${'SUBMIT'}: $filesWithDNS'); - - final bodyContainsDNS = body.contains('DO_NOT${'_'}SUBMIT'); - print('The body contains a DO_NOT${'_'}SUBMIT string: $bodyContainsDNS'); + .where((file) => File(file.relativePath).existsSync()) + .where((file) { + return File(file.relativePath).readAsStringSync().contains(dns); + }).toList(); + print('Found files with $dns: $filesWithDNS'); + + 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')} '''; From 1e5ebf21cef5e3616ee09157bff5e6bb93277aea Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:14:56 +0100 Subject: [PATCH 03/11] Add default --- pkgs/firehose/bin/health.dart | 1 + pkgs/firehose/lib/src/health/health.dart | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index cbf6bd21..ba78fcba 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -26,6 +26,7 @@ void main(List arguments) async { ) ..addMultiOption( 'experiments', + defaultsTo: [], help: 'Which experiments should be enabled for Dart', ) ..addFlag( diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 5acd7e51..5b06e96c 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -270,9 +270,9 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst .where((file) => ![FileStatus.removed, FileStatus.unchanged].contains(file.status)) .where((file) => File(file.relativePath).existsSync()) - .where((file) { - return File(file.relativePath).readAsStringSync().contains(dns); - }).toList(); + .where( + (file) => File(file.relativePath).readAsStringSync().contains(dns)) + .toList(); print('Found files with $dns: $filesWithDNS'); final bodyContainsDNS = body.contains(dns); From f2e777ece21ead101414b0127d2ac13aa44baac1 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:20:35 +0100 Subject: [PATCH 04/11] Add default to yaml --- .github/workflows/health_base.yaml | 2 +- pkgs/firehose/bin/health.dart | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index c9ab800a..1a4e7dae 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -57,7 +57,7 @@ on: type: boolean experiments: description: Which experiments should be enabled for Dart. - default: "" + default: "\"\"" type: string required: false diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index ba78fcba..cbf6bd21 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -26,7 +26,6 @@ void main(List arguments) async { ) ..addMultiOption( 'experiments', - defaultsTo: [], help: 'Which experiments should be enabled for Dart', ) ..addFlag( From 75ad54672fe2e7ccbe4087bccfe4ed705358dfc4 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:23:01 +0100 Subject: [PATCH 05/11] Add default to other wf --- .github/workflows/health.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 2d07db6c..751bcedb 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -90,7 +90,7 @@ on: type: boolean experiments: description: Which experiments should be enabled for Dart. - default: "" + default: "\"\"" type: string required: false From 21ab1ecfd4122ef09eb981d3024402fa38fbb441 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:29:08 +0100 Subject: [PATCH 06/11] Add debug message --- pkgs/firehose/lib/src/health/coverage.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index b2513180..dce2fd47 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -86,6 +86,8 @@ class Coverage { } Map getCoverage(Package? package) { + print( + 'Experiments: $experiments, ${experiments.length}, ${experiments.map((e) => e.codeUnits).toList()}'); if (package != null) { var hasTests = Directory(path.join(package.directory.path, 'test')).existsSync(); From 9e5467de05a7db3aae4753d66c2c0c343fdaaf8a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:36:32 +0100 Subject: [PATCH 07/11] Remove empty from list --- pkgs/firehose/bin/health.dart | 4 +++- pkgs/firehose/lib/src/health/coverage.dart | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index cbf6bd21..f7f75f96 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -36,7 +36,9 @@ void main(List arguments) async { var check = parsedArgs['check'] as String; var warnOn = parsedArgs['warn_on'] as List; var failOn = parsedArgs['fail_on'] as List; - var experiments = parsedArgs['experiments'] as List; + var experiments = (parsedArgs['experiments'] as List) + .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 ' diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index dce2fd47..b2513180 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -86,8 +86,6 @@ class Coverage { } Map getCoverage(Package? package) { - print( - 'Experiments: $experiments, ${experiments.length}, ${experiments.map((e) => e.codeUnits).toList()}'); if (package != null) { var hasTests = Directory(path.join(package.directory.path, 'test')).existsSync(); From d311825799c8df89386d4d5cf51079c7b4a80f1e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:38:33 +0100 Subject: [PATCH 08/11] Add empty arg --- 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 beb865c6..2cfbeaa2 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); + FakeHealth() : super(true, []); @override Map getCoverage(Package? package) { From 8a7453d29bccf4d4b65f1745f06556027c6c8c3d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:42:14 +0100 Subject: [PATCH 09/11] Rev version --- pkgs/firehose/CHANGELOG.md | 4 ++++ pkgs/firehose/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index d54df277..0e106d55 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.5.3 + +- Allow experiments to be enabled for Dart. + ## 0.5.2 - Also run health workflows on bot PRs. diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml index 4519fdec..de9ff9ba 100644 --- a/pkgs/firehose/pubspec.yaml +++ b/pkgs/firehose/pubspec.yaml @@ -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: From 8bfff5743d4900d96d5491b5b8ffd22d22683c6b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 11:44:07 +0100 Subject: [PATCH 10/11] join by comma --- pkgs/firehose/lib/src/health/coverage.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index b2513180..d4fb1608 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -95,7 +95,8 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path Process.runSync( 'dart', [ - if (experiments.isNotEmpty) 'enable-experiment=$experiments', + if (experiments.isNotEmpty) + 'enable-experiment=${experiments.join(',')}', 'pub', 'get' ], @@ -106,7 +107,8 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path var resultChrome = Process.runSync( 'dart', [ - if (experiments.isNotEmpty) 'enable-experiment=$experiments', + if (experiments.isNotEmpty) + 'enable-experiment=${experiments.join(',')}', 'test', '-p', 'chrome', @@ -121,7 +123,8 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path var resultVm = Process.runSync( 'dart', [ - if (experiments.isNotEmpty) 'enable-experiment=$experiments', + if (experiments.isNotEmpty) + 'enable-experiment=${experiments.join(',')}', 'test', '--coverage=coverage' ], From a637a43c3fedfd27ed9654a2f39d9d8c0c8f9f31 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jan 2024 12:06:24 +0100 Subject: [PATCH 11/11] Fix typos --- pkgs/firehose/lib/src/health/coverage.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart index d4fb1608..8e617f26 100644 --- a/pkgs/firehose/lib/src/health/coverage.dart +++ b/pkgs/firehose/lib/src/health/coverage.dart @@ -96,7 +96,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path 'dart', [ if (experiments.isNotEmpty) - 'enable-experiment=${experiments.join(',')}', + '--enable-experiment=${experiments.join(',')}', 'pub', 'get' ], @@ -108,7 +108,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path 'dart', [ if (experiments.isNotEmpty) - 'enable-experiment=${experiments.join(',')}', + '--enable-experiment=${experiments.join(',')}', 'test', '-p', 'chrome', @@ -124,7 +124,7 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path 'dart', [ if (experiments.isNotEmpty) - 'enable-experiment=${experiments.join(',')}', + '--enable-experiment=${experiments.join(',')}', 'test', '--coverage=coverage' ],