From 41de2b5ed88b3f99262ef05096c0120f15829c40 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 12:09:26 -0700 Subject: [PATCH 1/7] feat(wait-action): add wait_for_tests shared GitHub Action and CLI tool Add the wait_for_tests package to the Cocoon workspace. This package provides a shared GitHub Action and a companion CLI tool written in Dart that polls the Cocoon API until requested pre-submit tests are complete. Key additions: - Created the wait_for_tests CLI tool in `bin/wait_for_tests.dart` supporting environment-variable or command-line arguments, strict 40-character hexadecimal SHA validation, and help outputs. - Implemented robust polling, status clamping (between 30 and 600 seconds), and evaluation logic in `lib/wait_for_tests.dart` using the shared `TaskStatus` from `package:cocoon_common`. - Configured the GitHub Action workflow definition in `action.yml`. - Added comprehensive unit tests in `test/wait_for_tests_test.dart` and integration/CLI tests in `test/bin_test.dart` covering API error handling, interval clamping, and argument parsing. - Registered the package in the workspace root `pubspec.yaml`. --- packages/wait_for_tests/action.yml | 31 + .../wait_for_tests/bin/wait_for_tests.dart | 172 ++++ .../wait_for_tests/lib/wait_for_tests.dart | 263 ++++++ packages/wait_for_tests/pubspec.yaml | 21 + packages/wait_for_tests/test/bin_test.dart | 50 ++ .../test/wait_for_tests_test.dart | 784 ++++++++++++++++++ pubspec.yaml | 1 + 7 files changed, 1322 insertions(+) create mode 100644 packages/wait_for_tests/action.yml create mode 100644 packages/wait_for_tests/bin/wait_for_tests.dart create mode 100644 packages/wait_for_tests/lib/wait_for_tests.dart create mode 100644 packages/wait_for_tests/pubspec.yaml create mode 100644 packages/wait_for_tests/test/bin_test.dart create mode 100644 packages/wait_for_tests/test/wait_for_tests_test.dart diff --git a/packages/wait_for_tests/action.yml b/packages/wait_for_tests/action.yml new file mode 100644 index 0000000000..d8bb3c32a7 --- /dev/null +++ b/packages/wait_for_tests/action.yml @@ -0,0 +1,31 @@ +name: 'Wait for Cocoon Tests' +description: 'Polls the Cocoon API and waits for a specific list of pre-submit / merge queue Cocoon tests, to complete `successfully` or `neutral`' +author: 'The Flutter Authors' +inputs: + sha: + description: 'The commit SHA to poll' + required: true + repo: + description: 'The repository slug (e.g., flutter or packages)' + required: true + required-tests: + description: 'Comma or newline-separated list of tests to wait for (optional, if omitted waits for all scheduled tests)' + required: false + wait-interval: + description: 'Time in seconds between polling checks (min: 30, max: 600)' + required: false + default: '60' +runs: + using: 'composite' + steps: + - name: Get dependencies + shell: bash + run: dart pub get --directory=${{ github.action_path }} + - name: Run wait-for-tests + shell: bash + env: + INPUT_SHA: ${{ inputs.sha }} + INPUT_REPO: ${{ inputs.repo }} + INPUT_REQUIRED_TESTS: ${{ inputs.required-tests }} + INPUT_WAIT_INTERVAL: ${{ inputs.wait-interval }} + run: dart ${{ github.action_path }}/bin/wait_for_tests.dart diff --git a/packages/wait_for_tests/bin/wait_for_tests.dart b/packages/wait_for_tests/bin/wait_for_tests.dart new file mode 100644 index 0000000000..8e6a06cbf8 --- /dev/null +++ b/packages/wait_for_tests/bin/wait_for_tests.dart @@ -0,0 +1,172 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; +import 'package:args/args.dart'; +import 'package:http/http.dart' as http; +import 'package:wait_for_tests/wait_for_tests.dart'; + +/// Runs the wait-for-tests tool to poll the Cocoon API until the requested pre-submit tests finish. +/// +/// This CLI can be executed by passing options as command-line arguments: +/// ```sh +/// dart bin/wait_for_tests.dart --sha d100ca3882520e04129ff2a5c09372ecec3b3860 --repo flutter --required-tests "Linux windows_host_engine, Mac mac_ios_engine" --wait-interval 45 +/// ``` +/// Alternatively, it can read from environment variables (e.g., prefixed with `INPUT_` for GitHub Actions). +void main(List arguments) async { + final parser = ArgParser() + ..addOption('sha', abbr: 's', help: 'The full 40-character commit SHA to poll') + ..addOption( + 'repo', + abbr: 'r', + help: 'The repository slug or name (e.g., flutter or packages)', + ) + ..addMultiOption( + 'required-tests', + abbr: 't', + help: + 'Comma, newline, or repeatedly specified list of tests to wait for. If omitted, polls and waits for all scheduled tests.', + ) + ..addOption( + 'wait-interval', + abbr: 'i', + defaultsTo: '60', + help: 'Time in seconds between polling checks (clamped to a range of 30 to 600 seconds)', + ) + ..addOption( + 'owner', + abbr: 'o', + defaultsTo: 'flutter', + help: 'The repository owner', + ) + ..addFlag( + 'help', + abbr: 'h', + negatable: false, + help: 'Show usage information', + ); + + final ArgResults argResults; + try { + argResults = parser.parse(arguments); + } catch (e) { + stderr.writeln('Error parsing arguments: $e'); + _printUsage(parser); + exit(1); + } + + if (argResults['help'] as bool) { + _printUsage(parser); + exit(0); + } + + // Support reading from environment variables for GitHub Actions + final sha = argResults['sha'] as String? ?? Platform.environment['INPUT_SHA']; + var repo = + argResults['repo'] as String? ?? Platform.environment['INPUT_REPO']; + + final rawRequiredTestsList = + argResults['required-tests'] as List? ?? []; + final envRequiredTests = + Platform.environment['INPUT_REQUIRED_TESTS'] ?? + Platform.environment['INPUT_REQUIRED-TESTS']; + + final waitIntervalStr = + argResults['wait-interval'] as String? ?? + Platform.environment['INPUT_WAIT_INTERVAL'] ?? + Platform.environment['INPUT_WAIT-INTERVAL'] ?? + '60'; + var owner = + argResults['owner'] as String? ?? + Platform.environment['INPUT_OWNER'] ?? + 'flutter'; + + if (sha == null || sha.isEmpty) { + stderr.writeln( + 'Error: The commit "sha" parameter is required (either via CLI argument or INPUT_SHA environment variable).', + ); + _printUsage(parser); + exit(1); + } + + final shaRegex = RegExp(r'^[a-fA-F0-9]{40}$'); + if (!shaRegex.hasMatch(sha)) { + stderr.writeln( + 'Error: The commit "sha" parameter must be a full 40-character hexadecimal SHA. Got: "$sha"', + ); + exit(1); + } + + if (repo == null || repo.isEmpty) { + stderr.writeln( + 'Error: The "repo" parameter is required (either via CLI argument or INPUT_REPO environment variable).', + ); + _printUsage(parser); + exit(1); + } + + // Parse repo slug (e.g., "flutter/packages" or just "packages") + if (repo.contains('/')) { + final parts = repo.split('/'); + owner = parts[0]; + repo = parts[1]; + } + + final List requiredTests; + if (rawRequiredTestsList.isNotEmpty) { + requiredTests = rawRequiredTestsList + .expand((s) => s.split(RegExp(r'[,\n]'))) + .map((s) => s.trim()) + .where((s) => s.isNotEmpty) + .toList(); + } else if (envRequiredTests != null && envRequiredTests.isNotEmpty) { + requiredTests = envRequiredTests + .split(RegExp(r'[,\n]')) + .map((s) => s.trim()) + .where((s) => s.isNotEmpty) + .toList(); + } else { + requiredTests = const []; + } + + final waitIntervalSeconds = int.tryParse(waitIntervalStr); + if (waitIntervalSeconds == null) { + stderr.writeln( + 'Error: "wait-interval" must be a valid integer. Got: $waitIntervalStr', + ); + exit(1); + } + + final waitInterval = Duration(seconds: waitIntervalSeconds); + + final client = http.Client(); + try { + final success = await waitForTests( + sha: sha, + repo: repo, + requiredTests: requiredTests, + waitInterval: waitInterval, + client: client, + log: stdout.writeln, + owner: owner, + ); + + if (success) { + stdout.writeln('Success: Action completed successfully!'); + exit(0); + } else { + stderr.writeln('Failure: Action completed with errors.'); + exit(1); + } + } finally { + client.close(); + } +} + +void _printUsage(ArgParser parser) { + stdout.writeln('Usage: wait-for-tests [options]\n'); + stdout.writeln('This command-line tool can be configured either via command-line arguments'); + stdout.writeln('or environment variables (e.g., prefixed with INPUT_ for GitHub Actions).\n'); + stdout.writeln(parser.usage); +} diff --git a/packages/wait_for_tests/lib/wait_for_tests.dart b/packages/wait_for_tests/lib/wait_for_tests.dart new file mode 100644 index 0000000000..235a27055b --- /dev/null +++ b/packages/wait_for_tests/lib/wait_for_tests.dart @@ -0,0 +1,263 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:convert'; +import 'package:clock/clock.dart'; +import 'package:cocoon_common/task_status.dart'; +import 'package:http/http.dart' as http; + +/// A structured summary for a specific required test. +class TestStatusSummary { + final String name; + final TaskStatus status; + final String originalStatusString; + + const TestStatusSummary({ + required this.name, + required this.status, + required this.originalStatusString, + }); +} + +/// The aggregated result of a polling check. +class PollResult { + final bool allSucceeded; + final bool anyFailed; + final List summaries; + final String guardStatus; + final int remainingCount; + + const PollResult({ + required this.allSucceeded, + required this.anyFailed, + required this.summaries, + required this.guardStatus, + required this.remainingCount, + }); +} + +/// Evaluates the status of the required tests based on the Cocoon API response. +PollResult evaluateTests({ + required Map json, + required List requiredTests, +}) { + final allJobs = {}; + if (json case {'stages': final List stages}) { + for (final stage in stages) { + if (stage case {'jobs': final Map jobs}) { + for (final MapEntry(key: String key, value: String value) + in jobs.entries) { + allJobs[key] = value; + } + } + } + } + + // Pre-build a normalized lookup map to avoid O(M * N) iteration + final normalizedJobs = { + for (final MapEntry(key: name, value: status) in allJobs.entries) + name.trim().toLowerCase(): (name, status), + }; + + // If requiredTests is empty, evaluate all available jobs + final targetTests = requiredTests.isNotEmpty + ? requiredTests + : allJobs.keys.toList(); + + final summaries = []; + final guardStatusStr = json['guard_status'] as String? ?? ''; + final isGuardFailed = switch (guardStatusStr.toLowerCase().replaceAll( + ' ', + '', + )) { + 'failed' || + 'infra_failure' || + 'infrafailure' || + 'cancelled' || + 'canceled' => true, + _ => false, + }; + + var allSucceeded = true; + var anyFailed = isGuardFailed; + + for (final targetTest in targetTests) { + final trimmedName = targetTest.trim(); + if (trimmedName.isEmpty) continue; + + final lookup = normalizedJobs[trimmedName.toLowerCase()]; + final (matchedJobName, originalStatus) = lookup ?? (null, null); + + final TaskStatus status; + final String statusStr; + + if (originalStatus != null) { + status = _parseStatus(originalStatus); + statusStr = originalStatus; + } else { + status = TaskStatus.waitingForBackfill; + statusStr = 'Not yet scheduled'; + } + + if (!status.isSuccess) { + allSucceeded = false; + } + if (status.isFailure) { + anyFailed = true; + } + + summaries.add( + TestStatusSummary( + name: matchedJobName ?? trimmedName, + status: status, + originalStatusString: statusStr, + ), + ); + } + + final remainingCount = summaries + .where((s) => s.status.isBuildInProgress) + .length; + + return PollResult( + allSucceeded: allSucceeded, + anyFailed: anyFailed, + summaries: summaries, + guardStatus: guardStatusStr, + remainingCount: remainingCount, + ); +} + +TaskStatus _parseStatus(String statusStr) { + return switch (statusStr.toLowerCase().replaceAll(' ', '')) { + 'succeeded' || 'success' => TaskStatus.succeeded, + 'neutral' => TaskStatus.neutral, + 'skipped' => TaskStatus.skipped, + 'failed' => TaskStatus.failed, + 'infra_failure' || 'infrafailure' => TaskStatus.infraFailure, + 'cancelled' || 'canceled' => TaskStatus.cancelled, + 'inprogress' || 'in_progress' || 'running' => TaskStatus.inProgress, + 'new' || + 'pending' || + 'waiting' || + 'queued' || + 'scheduled' => TaskStatus.waitingForBackfill, + _ => TaskStatus.waitingForBackfill, + }; +} + +/// Polls the Cocoon API until all required tests complete or any fails. +/// Returns true if all tests succeeded, false if any failed or loop timed out. +Future waitForTests({ + required String sha, + required String repo, + required List requiredTests, + required Duration waitInterval, + required http.Client client, + required void Function(String) log, + Duration? timeout, + String owner = 'flutter', +}) async { + final clampedSeconds = waitInterval.inSeconds.clamp(30, 600); + final clampedWaitInterval = clampedSeconds != waitInterval.inSeconds + ? Duration(seconds: clampedSeconds) + : waitInterval; + + final startTime = clock.now(); + final url = Uri.https( + 'flutter-dashboard.appspot.com', + '/api/public/get-presubmit-guard', + {'sha': sha, 'repo': repo, 'owner': owner}, + ); + + log('Starting wait-for-tests polling loop.'); + log('Repository: $owner/$repo, Commit SHA: $sha'); + if (clampedSeconds != waitInterval.inSeconds) { + log( + 'Warning: wait-interval must be between 30 and 600 seconds. Clamping from ${waitInterval.inSeconds}s to ${clampedSeconds}s.', + ); + } + + if (requiredTests.isEmpty) { + log('Required tests to wait for: [All scheduled tests]'); + } else { + log('Required tests to wait for: ${requiredTests.join(", ")}'); + } + log('Wait interval: ${clampedWaitInterval.inSeconds} seconds'); + + while (true) { + if (timeout != null && clock.now().difference(startTime) > timeout) { + log('Error: Polling timed out after ${timeout.inMinutes} minutes.'); + return false; + } + + try { + final response = await client.get(url); + if (response.statusCode != 200) { + log( + 'Warning: Cocoon API returned status code ${response.statusCode}. Body: ${response.body}', + ); + } else { + Map json; + try { + json = jsonDecode(response.body) as Map; + } catch (e) { + log('Warning: Failed to parse Cocoon API response as JSON: $e'); + json = {}; + } + + if (json.isNotEmpty) { + final result = evaluateTests( + json: json, + requiredTests: requiredTests, + ); + + log('\n--- Current Test Status Summary ---'); + for (final summary in result.summaries) { + log( + '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', + ); + } + if (requiredTests.isEmpty) { + log('Remaining tests: ${result.remainingCount}'); + log('Overall Guard Status: ${result.guardStatus}'); + } + log('-----------------------------------\n'); + + final bool isSuccess; + if (requiredTests.isEmpty) { + isSuccess = + result.allSucceeded && + result.guardStatus.toLowerCase() == 'succeeded'; + } else { + isSuccess = result.allSucceeded; + } + + if (isSuccess) { + if (requiredTests.isEmpty) { + log( + 'Success: Overall guard status succeeded and all tests completed successfully!', + ); + } else { + log('Success: All required tests have completed successfully!'); + } + return true; + } + + if (result.anyFailed) { + log( + 'Error: One or more tests have failed, or the overall guard status is failed.', + ); + return false; + } + } + } + } catch (e) { + log('Warning: Error calling Cocoon API: $e'); + } + + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + } +} diff --git a/packages/wait_for_tests/pubspec.yaml b/packages/wait_for_tests/pubspec.yaml new file mode 100644 index 0000000000..9b65bf5fba --- /dev/null +++ b/packages/wait_for_tests/pubspec.yaml @@ -0,0 +1,21 @@ +name: wait_for_tests +description: A shared GitHub Action written in Dart to wait for specific Cocoon pre-submit tests to complete. +publish_to: none + +environment: + sdk: ^3.10.0 + +resolution: workspace + +dependencies: + args: ^2.6.0 + clock: ^1.1.2 + cocoon_common: + path: ../cocoon_common + http: ^1.2.1 + meta: ^1.16.0 + +dev_dependencies: + dart_flutter_team_lints: ^3.5.2 + fake_async: ^1.3.1 + test: ^1.25.15 diff --git a/packages/wait_for_tests/test/bin_test.dart b/packages/wait_for_tests/test/bin_test.dart new file mode 100644 index 0000000000..17a2af8fe4 --- /dev/null +++ b/packages/wait_for_tests/test/bin_test.dart @@ -0,0 +1,50 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; +import 'package:test/test.dart'; + +void main() { + final binPath = 'bin/wait_for_tests.dart'; + + test('prints usage when --help is passed', () async { + final result = await Process.run('dart', [binPath, '--help']); + expect(result.exitCode, equals(0)); + expect(result.stdout, contains('Usage: wait-for-tests [options]')); + expect(result.stdout, contains('--sha')); + expect(result.stdout, contains('--repo')); + }); + + test('exits with code 1 and prints error when missing required sha and repo', () async { + final result = await Process.run('dart', [binPath]); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: The commit "sha" parameter is required')); + }); + + test('exits with code 1 and prints error when missing required repo', () async { + final result = await Process.run('dart', [binPath, '--sha', 'd100ca3882520e04129ff2a5c09372ecec3b3860']); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: The "repo" parameter is required')); + }); + + test('exits with code 1 and prints error when wait-interval is not an integer', () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + 'd100ca3882520e04129ff2a5c09372ecec3b3860', + '--repo', + 'flutter', + '--wait-interval', + 'abc', + ]); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: "wait-interval" must be a valid integer. Got: abc')); + }); + + test('exits with code 1 and prints error when sha is not a full 40-character hex string', () async { + final result = await Process.run('dart', [binPath, '--sha', '3b77a01', '--repo', 'flutter']); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: The commit "sha" parameter must be a full 40-character hexadecimal SHA')); + }); +} diff --git a/packages/wait_for_tests/test/wait_for_tests_test.dart b/packages/wait_for_tests/test/wait_for_tests_test.dart new file mode 100644 index 0000000000..3f55fa5e71 --- /dev/null +++ b/packages/wait_for_tests/test/wait_for_tests_test.dart @@ -0,0 +1,784 @@ +import 'dart:convert'; +import 'package:cocoon_common/task_status.dart'; +import 'package:fake_async/fake_async.dart'; +import 'package:http/http.dart' as http; +import 'package:test/test.dart'; +import 'package:wait_for_tests/wait_for_tests.dart'; + +class MockClient extends http.BaseClient { + final Future Function(http.BaseRequest) _sendHandler; + + MockClient(this._sendHandler); + + @override + Future send(http.BaseRequest request) { + return _sendHandler(request); + } +} + +http.StreamedResponse _stringResponse(String body, int status) { + return http.StreamedResponse( + Stream.value(utf8.encode(body)), + status, + headers: {'content-type': 'application/json'}, + ); +} + +void main() { + group('evaluateTests', () { + test('succeeds when all required tests succeed', () { + final json = { + 'stages': [ + { + 'jobs': { + 'Linux windows_host_engine': 'Succeeded', + 'Mac mac_ios_engine': 'Neutral', + 'Linux linux_fuchsia': 'Skipped', + }, + }, + ], + }; + final requiredTests = [ + 'Linux windows_host_engine', + 'Mac mac_ios_engine', + 'Linux linux_fuchsia', + ]; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isTrue); + expect(result.anyFailed, isFalse); + expect(result.summaries, hasLength(3)); + expect(result.summaries[0].status, TaskStatus.succeeded); + expect(result.summaries[1].status, TaskStatus.neutral); + expect(result.summaries[2].status, TaskStatus.skipped); + }); + + test('fails when any required test fails', () { + final json = { + 'stages': [ + { + 'jobs': { + 'Linux windows_host_engine': 'Succeeded', + 'Mac mac_ios_engine': 'Failed', + }, + }, + ], + }; + final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isFalse); + expect(result.anyFailed, isTrue); + expect(result.summaries[1].status, TaskStatus.failed); + }); + + test('is pending when some tests are in progress or waiting', () { + final json = { + 'stages': [ + { + 'jobs': { + 'Linux windows_host_engine': 'Succeeded', + 'Mac mac_ios_engine': 'In Progress', + }, + }, + ], + }; + final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isFalse); + expect(result.anyFailed, isFalse); + expect(result.summaries[1].status, TaskStatus.inProgress); + }); + + test('treats missing required tests as waiting', () { + final json = { + 'stages': [ + { + 'jobs': {'Linux windows_host_engine': 'Succeeded'}, + }, + ], + }; + final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isFalse); + expect(result.anyFailed, isFalse); + expect(result.summaries[1].status, TaskStatus.waitingForBackfill); + expect(result.summaries[1].originalStatusString, 'Not yet scheduled'); + }); + + test('is case-insensitive and trims whitespace on job name matching', () { + final json = { + 'stages': [ + { + 'jobs': {' Linux windows_host_engine ': 'Succeeded'}, + }, + ], + }; + final requiredTests = ['linux windows_host_engine']; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isTrue); + expect(result.anyFailed, isFalse); + }); + + test('correctly parses various status string variations', () { + final statusMap = { + 'success': TaskStatus.succeeded, + 'succeeded': TaskStatus.succeeded, + 'neutral': TaskStatus.neutral, + 'skipped': TaskStatus.skipped, + 'failed': TaskStatus.failed, + 'infra_failure': TaskStatus.infraFailure, + 'infrafailure': TaskStatus.infraFailure, + 'cancelled': TaskStatus.cancelled, + 'canceled': TaskStatus.cancelled, + 'inprogress': TaskStatus.inProgress, + 'in_progress': TaskStatus.inProgress, + 'running': TaskStatus.inProgress, + 'new': TaskStatus.waitingForBackfill, + 'pending': TaskStatus.waitingForBackfill, + 'waiting': TaskStatus.waitingForBackfill, + 'queued': TaskStatus.waitingForBackfill, + 'scheduled': TaskStatus.waitingForBackfill, + 'some_weird_unsupported_status': TaskStatus.waitingForBackfill, + }; + + for (final MapEntry(key: inputStatus, value: expectedStatus) + in statusMap.entries) { + final json = { + 'stages': [ + { + 'jobs': {'test_job': inputStatus}, + }, + ], + }; + final result = evaluateTests(json: json, requiredTests: ['test_job']); + expect( + result.summaries[0].status, + expectedStatus, + reason: 'Failed to parse "$inputStatus" to $expectedStatus', + ); + } + }); + + test( + 'isGuardFailed causes anyFailed to be true even when requiredTests is provided and has no failed tests', + () { + final json = { + 'guard_status': 'infra_failure', + 'stages': [ + { + 'jobs': {'Linux windows_host_engine': 'Succeeded'}, + }, + ], + }; + final requiredTests = ['Linux windows_host_engine']; + final result = evaluateTests(json: json, requiredTests: requiredTests); + + expect(result.allSucceeded, isTrue); + expect(result.anyFailed, isTrue); + }, + ); + }); + + group('waitForTests with fake_async', () { + test('returns immediately on success on first check', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 10), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isTrue); + }); + }); + + test('returns immediately on failure on first check', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Failed" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = true; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 10), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isFalse); + }); + }); + + test('polls multiple times and succeeds once tests complete', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "In progress" + } + } + ] + } + ''', 200); + } else { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll happens immediately + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + + // elapse by interval to trigger second poll + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('polls multiple times and fails once a test fails', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "In progress" + } + } + ] + } + ''', 200); + } else { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Failed" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = true; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isFalse); + expect(callCount, 2); + }); + }); + + test('times out after configured duration', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "In progress" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = true; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: (_) {}, + timeout: const Duration(seconds: 15), + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(seconds: 35)); + + expect(completed, isTrue); + expect(successResult, isFalse); + }); + }); + + test('waits for all tests to succeed when requiredTests is empty', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse(''' + { + "guard_status": "In progress", + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded", + "Mac mac_ios_engine": "In progress" + } + } + ] + } + ''', 200); + } else { + return _stringResponse(''' + { + "guard_status": "Succeeded", + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded", + "Mac mac_ios_engine": "Succeeded" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const [], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + + // Second poll (after wait interval) + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('fails immediately when any test fails in all-tests mode', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "guard_status": "Failed", + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Failed", + "Mac mac_ios_engine": "Succeeded" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = true; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const [], + waitInterval: const Duration(seconds: 10), + client: mockClient, + log: (_) {}, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isFalse); + }); + }); + + test('continues polling when HTTP client throws exception', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + throw Exception('Transient network error'); + } else { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: fails with exception, caught, slept + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any((l) => l.contains('Warning: Error calling Cocoon API')), + isTrue, + ); + + // Advance to trigger second poll + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('continues polling when API returns non-200 status code', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse('Gateway Timeout', 504); + } else { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: 504 + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => l.contains('Warning: Cocoon API returned status code 504'), + ), + isTrue, + ); + + // Trigger second poll + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('continues polling when response is not valid JSON', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse('not a json map', 200); + } else { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: invalid JSON + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => l.contains( + 'Warning: Failed to parse Cocoon API response as JSON', + ), + ), + isTrue, + ); + + // Trigger second poll + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('clamps waitInterval to at least 30 seconds and logs warning', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 10), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect( + logs.any( + (l) => l.contains( + 'Warning: wait-interval must be between 30 and 600 seconds. Clamping from 10s to 30s.', + ), + ), + isTrue, + ); + }); + }); + + test('clamps waitInterval to at most 600 seconds and logs warning', () { + final mockClient = MockClient((request) async { + return _stringResponse(''' + { + "stages": [ + { + "jobs": { + "Linux windows_host_engine": "Succeeded" + } + } + ] + } + ''', 200); + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + repo: 'flutter', + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 1000), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect( + logs.any( + (l) => l.contains( + 'Warning: wait-interval must be between 30 and 600 seconds. Clamping from 1000s to 600s.', + ), + ), + isTrue, + ); + }); + }); + }); +} diff --git a/pubspec.yaml b/pubspec.yaml index 73e1c03738..f017698f2f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -20,6 +20,7 @@ workspace: - packages/cocoon_server - packages/cocoon_server_test - packages/cocoon_integration_test + - packages/wait_for_tests - dev/cocoon_code_health - dev/githubanalysis From 744bbbf4956682db31782ac534f04d6de41f49f2 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 12:14:02 -0700 Subject: [PATCH 2/7] Format --- .../wait_for_tests/bin/wait_for_tests.dart | 17 +++- packages/wait_for_tests/test/bin_test.dart | 92 +++++++++++++------ 2 files changed, 77 insertions(+), 32 deletions(-) diff --git a/packages/wait_for_tests/bin/wait_for_tests.dart b/packages/wait_for_tests/bin/wait_for_tests.dart index 8e6a06cbf8..6f6edd8a54 100644 --- a/packages/wait_for_tests/bin/wait_for_tests.dart +++ b/packages/wait_for_tests/bin/wait_for_tests.dart @@ -16,7 +16,11 @@ import 'package:wait_for_tests/wait_for_tests.dart'; /// Alternatively, it can read from environment variables (e.g., prefixed with `INPUT_` for GitHub Actions). void main(List arguments) async { final parser = ArgParser() - ..addOption('sha', abbr: 's', help: 'The full 40-character commit SHA to poll') + ..addOption( + 'sha', + abbr: 's', + help: 'The full 40-character commit SHA to poll', + ) ..addOption( 'repo', abbr: 'r', @@ -32,7 +36,8 @@ void main(List arguments) async { 'wait-interval', abbr: 'i', defaultsTo: '60', - help: 'Time in seconds between polling checks (clamped to a range of 30 to 600 seconds)', + help: + 'Time in seconds between polling checks (clamped to a range of 30 to 600 seconds)', ) ..addOption( 'owner', @@ -166,7 +171,11 @@ void main(List arguments) async { void _printUsage(ArgParser parser) { stdout.writeln('Usage: wait-for-tests [options]\n'); - stdout.writeln('This command-line tool can be configured either via command-line arguments'); - stdout.writeln('or environment variables (e.g., prefixed with INPUT_ for GitHub Actions).\n'); + stdout.writeln( + 'This command-line tool can be configured either via command-line arguments', + ); + stdout.writeln( + 'or environment variables (e.g., prefixed with INPUT_ for GitHub Actions).\n', + ); stdout.writeln(parser.usage); } diff --git a/packages/wait_for_tests/test/bin_test.dart b/packages/wait_for_tests/test/bin_test.dart index 17a2af8fe4..4ff2cb78ee 100644 --- a/packages/wait_for_tests/test/bin_test.dart +++ b/packages/wait_for_tests/test/bin_test.dart @@ -16,35 +16,71 @@ void main() { expect(result.stdout, contains('--repo')); }); - test('exits with code 1 and prints error when missing required sha and repo', () async { - final result = await Process.run('dart', [binPath]); - expect(result.exitCode, equals(1)); - expect(result.stderr, contains('Error: The commit "sha" parameter is required')); - }); + test( + 'exits with code 1 and prints error when missing required sha and repo', + () async { + final result = await Process.run('dart', [binPath]); + expect(result.exitCode, equals(1)); + expect( + result.stderr, + contains('Error: The commit "sha" parameter is required'), + ); + }, + ); - test('exits with code 1 and prints error when missing required repo', () async { - final result = await Process.run('dart', [binPath, '--sha', 'd100ca3882520e04129ff2a5c09372ecec3b3860']); - expect(result.exitCode, equals(1)); - expect(result.stderr, contains('Error: The "repo" parameter is required')); - }); + test( + 'exits with code 1 and prints error when missing required repo', + () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + 'd100ca3882520e04129ff2a5c09372ecec3b3860', + ]); + expect(result.exitCode, equals(1)); + expect( + result.stderr, + contains('Error: The "repo" parameter is required'), + ); + }, + ); - test('exits with code 1 and prints error when wait-interval is not an integer', () async { - final result = await Process.run('dart', [ - binPath, - '--sha', - 'd100ca3882520e04129ff2a5c09372ecec3b3860', - '--repo', - 'flutter', - '--wait-interval', - 'abc', - ]); - expect(result.exitCode, equals(1)); - expect(result.stderr, contains('Error: "wait-interval" must be a valid integer. Got: abc')); - }); + test( + 'exits with code 1 and prints error when wait-interval is not an integer', + () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + 'd100ca3882520e04129ff2a5c09372ecec3b3860', + '--repo', + 'flutter', + '--wait-interval', + 'abc', + ]); + expect(result.exitCode, equals(1)); + expect( + result.stderr, + contains('Error: "wait-interval" must be a valid integer. Got: abc'), + ); + }, + ); - test('exits with code 1 and prints error when sha is not a full 40-character hex string', () async { - final result = await Process.run('dart', [binPath, '--sha', '3b77a01', '--repo', 'flutter']); - expect(result.exitCode, equals(1)); - expect(result.stderr, contains('Error: The commit "sha" parameter must be a full 40-character hexadecimal SHA')); - }); + test( + 'exits with code 1 and prints error when sha is not a full 40-character hex string', + () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + '3b77a01', + '--repo', + 'flutter', + ]); + expect(result.exitCode, equals(1)); + expect( + result.stderr, + contains( + 'Error: The commit "sha" parameter must be a full 40-character hexadecimal SHA', + ), + ); + }, + ); } From 70ed6b5a2c463f72b112a57ea71540c25bcd5506 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 12:20:43 -0700 Subject: [PATCH 3/7] license --- packages/wait_for_tests/test/wait_for_tests_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/wait_for_tests/test/wait_for_tests_test.dart b/packages/wait_for_tests/test/wait_for_tests_test.dart index 3f55fa5e71..8ea64fc1fa 100644 --- a/packages/wait_for_tests/test/wait_for_tests_test.dart +++ b/packages/wait_for_tests/test/wait_for_tests_test.dart @@ -1,3 +1,7 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + import 'dart:convert'; import 'package:cocoon_common/task_status.dart'; import 'package:fake_async/fake_async.dart'; From 631ce7d59c1bf3917b34173f4e8f00faecdf57e3 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 13:45:31 -0700 Subject: [PATCH 4/7] cleanup --- .../wait_for_tests/bin/wait_for_tests.dart | 36 +- .../wait_for_tests/lib/wait_for_tests.dart | 100 ++- packages/wait_for_tests/pubspec.yaml | 1 + packages/wait_for_tests/test/bin_test.dart | 34 +- .../test/wait_for_tests_test.dart | 686 +++++++++++------- 5 files changed, 511 insertions(+), 346 deletions(-) diff --git a/packages/wait_for_tests/bin/wait_for_tests.dart b/packages/wait_for_tests/bin/wait_for_tests.dart index 6f6edd8a54..bad73fced8 100644 --- a/packages/wait_for_tests/bin/wait_for_tests.dart +++ b/packages/wait_for_tests/bin/wait_for_tests.dart @@ -4,6 +4,7 @@ import 'dart:io'; import 'package:args/args.dart'; +import 'package:github/github.dart'; import 'package:http/http.dart' as http; import 'package:wait_for_tests/wait_for_tests.dart'; @@ -11,7 +12,7 @@ import 'package:wait_for_tests/wait_for_tests.dart'; /// /// This CLI can be executed by passing options as command-line arguments: /// ```sh -/// dart bin/wait_for_tests.dart --sha d100ca3882520e04129ff2a5c09372ecec3b3860 --repo flutter --required-tests "Linux windows_host_engine, Mac mac_ios_engine" --wait-interval 45 +/// dart bin/wait_for_tests.dart --sha d100ca3882520e04129ff2a5c09372ecec3b3860 --repo flutter/flutter --required-tests "Linux windows_host_engine, Mac mac_ios_engine" --wait-interval 45 /// ``` /// Alternatively, it can read from environment variables (e.g., prefixed with `INPUT_` for GitHub Actions). void main(List arguments) async { @@ -24,7 +25,8 @@ void main(List arguments) async { ..addOption( 'repo', abbr: 'r', - help: 'The repository slug or name (e.g., flutter or packages)', + help: + 'The repository slug or name (e.g., flutter/flutter or flutter/packages)', ) ..addMultiOption( 'required-tests', @@ -39,12 +41,6 @@ void main(List arguments) async { help: 'Time in seconds between polling checks (clamped to a range of 30 to 600 seconds)', ) - ..addOption( - 'owner', - abbr: 'o', - defaultsTo: 'flutter', - help: 'The repository owner', - ) ..addFlag( 'help', abbr: 'h', @@ -68,7 +64,7 @@ void main(List arguments) async { // Support reading from environment variables for GitHub Actions final sha = argResults['sha'] as String? ?? Platform.environment['INPUT_SHA']; - var repo = + final repo = argResults['repo'] as String? ?? Platform.environment['INPUT_REPO']; final rawRequiredTestsList = @@ -82,10 +78,6 @@ void main(List arguments) async { Platform.environment['INPUT_WAIT_INTERVAL'] ?? Platform.environment['INPUT_WAIT-INTERVAL'] ?? '60'; - var owner = - argResults['owner'] as String? ?? - Platform.environment['INPUT_OWNER'] ?? - 'flutter'; if (sha == null || sha.isEmpty) { stderr.writeln( @@ -111,11 +103,18 @@ void main(List arguments) async { exit(1); } - // Parse repo slug (e.g., "flutter/packages" or just "packages") - if (repo.contains('/')) { + final RepositorySlug slug; + try { final parts = repo.split('/'); - owner = parts[0]; - repo = parts[1]; + if (parts.length != 2 || parts[0].isEmpty || parts[1].isEmpty) { + throw const FormatException(); + } + slug = RepositorySlug(parts[0], parts[1]); + } catch (_) { + stderr.writeln( + 'Error: Malformed "repo" parameter. Expected format "owner/repo" (e.g., "flutter/cocoon"), but got: "$repo"', + ); + exit(1); } final List requiredTests; @@ -149,12 +148,11 @@ void main(List arguments) async { try { final success = await waitForTests( sha: sha, - repo: repo, + slug: slug, requiredTests: requiredTests, waitInterval: waitInterval, client: client, log: stdout.writeln, - owner: owner, ); if (success) { diff --git a/packages/wait_for_tests/lib/wait_for_tests.dart b/packages/wait_for_tests/lib/wait_for_tests.dart index 235a27055b..1a461b4729 100644 --- a/packages/wait_for_tests/lib/wait_for_tests.dart +++ b/packages/wait_for_tests/lib/wait_for_tests.dart @@ -2,9 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:convert'; import 'package:clock/clock.dart'; +import 'package:cocoon_common/guard_status.dart'; +import 'package:cocoon_common/rpc_model.dart'; import 'package:cocoon_common/task_status.dart'; +import 'package:github/github.dart'; import 'package:http/http.dart' as http; /// A structured summary for a specific required test. @@ -39,18 +43,14 @@ class PollResult { /// Evaluates the status of the required tests based on the Cocoon API response. PollResult evaluateTests({ - required Map json, + required PresubmitGuardResponse response, required List requiredTests, }) { - final allJobs = {}; - if (json case {'stages': final List stages}) { - for (final stage in stages) { - if (stage case {'jobs': final Map jobs}) { - for (final MapEntry(key: String key, value: String value) - in jobs.entries) { - allJobs[key] = value; - } - } + final allJobs = {}; + for (final stage in response.stages) { + for (final MapEntry(key: String key, value: TaskStatus value) + in stage.jobs.entries) { + allJobs[key] = value; } } @@ -66,18 +66,7 @@ PollResult evaluateTests({ : allJobs.keys.toList(); final summaries = []; - final guardStatusStr = json['guard_status'] as String? ?? ''; - final isGuardFailed = switch (guardStatusStr.toLowerCase().replaceAll( - ' ', - '', - )) { - 'failed' || - 'infra_failure' || - 'infrafailure' || - 'cancelled' || - 'canceled' => true, - _ => false, - }; + final isGuardFailed = response.guardStatus == GuardStatus.failed; var allSucceeded = true; var anyFailed = isGuardFailed; @@ -93,8 +82,8 @@ PollResult evaluateTests({ final String statusStr; if (originalStatus != null) { - status = _parseStatus(originalStatus); - statusStr = originalStatus; + status = originalStatus; + statusStr = originalStatus.value; } else { status = TaskStatus.waitingForBackfill; statusStr = 'Not yet scheduled'; @@ -124,40 +113,21 @@ PollResult evaluateTests({ allSucceeded: allSucceeded, anyFailed: anyFailed, summaries: summaries, - guardStatus: guardStatusStr, + guardStatus: response.guardStatus.value, remainingCount: remainingCount, ); } -TaskStatus _parseStatus(String statusStr) { - return switch (statusStr.toLowerCase().replaceAll(' ', '')) { - 'succeeded' || 'success' => TaskStatus.succeeded, - 'neutral' => TaskStatus.neutral, - 'skipped' => TaskStatus.skipped, - 'failed' => TaskStatus.failed, - 'infra_failure' || 'infrafailure' => TaskStatus.infraFailure, - 'cancelled' || 'canceled' => TaskStatus.cancelled, - 'inprogress' || 'in_progress' || 'running' => TaskStatus.inProgress, - 'new' || - 'pending' || - 'waiting' || - 'queued' || - 'scheduled' => TaskStatus.waitingForBackfill, - _ => TaskStatus.waitingForBackfill, - }; -} - /// Polls the Cocoon API until all required tests complete or any fails. /// Returns true if all tests succeeded, false if any failed or loop timed out. Future waitForTests({ required String sha, - required String repo, + required RepositorySlug slug, required List requiredTests, required Duration waitInterval, required http.Client client, required void Function(String) log, Duration? timeout, - String owner = 'flutter', }) async { final clampedSeconds = waitInterval.inSeconds.clamp(30, 600); final clampedWaitInterval = clampedSeconds != waitInterval.inSeconds @@ -168,11 +138,11 @@ Future waitForTests({ final url = Uri.https( 'flutter-dashboard.appspot.com', '/api/public/get-presubmit-guard', - {'sha': sha, 'repo': repo, 'owner': owner}, + {'sha': sha, 'repo': slug.name, 'owner': slug.owner}, ); log('Starting wait-for-tests polling loop.'); - log('Repository: $owner/$repo, Commit SHA: $sha'); + log('Repository: $slug, Commit SHA: $sha'); if (clampedSeconds != waitInterval.inSeconds) { log( 'Warning: wait-interval must be between 30 and 600 seconds. Clamping from ${waitInterval.inSeconds}s to ${clampedSeconds}s.', @@ -193,23 +163,49 @@ Future waitForTests({ } try { - final response = await client.get(url); + final response = await client + .get(url) + .timeout(const Duration(seconds: 15)); if (response.statusCode != 200) { + final failImmediately = switch (response.statusCode) { + 429 || 408 => false, + >= 400 && < 500 => true, + _ => false, + }; + + if (failImmediately) { + log( + 'Error: Non-transient 4xx error from Cocoon API. Status code ${response.statusCode}. Body: ${response.body}', + ); + return false; + } log( 'Warning: Cocoon API returned status code ${response.statusCode}. Body: ${response.body}', ); } else { - Map json; + Map json; try { - json = jsonDecode(response.body) as Map; + json = jsonDecode(response.body) as Map; } catch (e) { log('Warning: Failed to parse Cocoon API response as JSON: $e'); - json = {}; + json = {}; } if (json.isNotEmpty) { + final PresubmitGuardResponse guardResponse; + try { + guardResponse = PresubmitGuardResponse.fromJson(json); + } catch (e) { + log( + 'Warning: Failed to parse JSON into PresubmitGuardResponse: $e', + ); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } + final result = evaluateTests( - json: json, + response: guardResponse, requiredTests: requiredTests, ); diff --git a/packages/wait_for_tests/pubspec.yaml b/packages/wait_for_tests/pubspec.yaml index 9b65bf5fba..8d0230df8e 100644 --- a/packages/wait_for_tests/pubspec.yaml +++ b/packages/wait_for_tests/pubspec.yaml @@ -12,6 +12,7 @@ dependencies: clock: ^1.1.2 cocoon_common: path: ../cocoon_common + github: ^9.25.0 http: ^1.2.1 meta: ^1.16.0 diff --git a/packages/wait_for_tests/test/bin_test.dart b/packages/wait_for_tests/test/bin_test.dart index 4ff2cb78ee..c27eb898cb 100644 --- a/packages/wait_for_tests/test/bin_test.dart +++ b/packages/wait_for_tests/test/bin_test.dart @@ -52,7 +52,7 @@ void main() { '--sha', 'd100ca3882520e04129ff2a5c09372ecec3b3860', '--repo', - 'flutter', + 'flutter/flutter', '--wait-interval', 'abc', ]); @@ -72,7 +72,7 @@ void main() { '--sha', '3b77a01', '--repo', - 'flutter', + 'flutter/flutter', ]); expect(result.exitCode, equals(1)); expect( @@ -83,4 +83,34 @@ void main() { ); }, ); + + test( + 'exits with code 1 and prints error when repo is malformed (e.g. trailing slash)', + () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + 'd100ca3882520e04129ff2a5c09372ecec3b3860', + '--repo', + 'flutter/', + ]); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: Malformed "repo" parameter')); + }, + ); + + test( + 'exits with code 1 and prints error when repo has too many slashes', + () async { + final result = await Process.run('dart', [ + binPath, + '--sha', + 'd100ca3882520e04129ff2a5c09372ecec3b3860', + '--repo', + 'flutter/packages/extra', + ]); + expect(result.exitCode, equals(1)); + expect(result.stderr, contains('Error: Malformed "repo" parameter')); + }, + ); } diff --git a/packages/wait_for_tests/test/wait_for_tests_test.dart b/packages/wait_for_tests/test/wait_for_tests_test.dart index 8ea64fc1fa..6e8aa824cf 100644 --- a/packages/wait_for_tests/test/wait_for_tests_test.dart +++ b/packages/wait_for_tests/test/wait_for_tests_test.dart @@ -3,8 +3,12 @@ // found in the LICENSE file. import 'dart:convert'; + +import 'package:cocoon_common/guard_status.dart'; +import 'package:cocoon_common/rpc_model.dart'; import 'package:cocoon_common/task_status.dart'; import 'package:fake_async/fake_async.dart'; +import 'package:github/github.dart'; import 'package:http/http.dart' as http; import 'package:test/test.dart'; import 'package:wait_for_tests/wait_for_tests.dart'; @@ -28,26 +32,55 @@ http.StreamedResponse _stringResponse(String body, int status) { ); } +/// Helper to create a strongly typed [PresubmitGuardResponse] for testing [evaluateTests]. +PresubmitGuardResponse _createResponse({ + required Map jobs, + GuardStatus guardStatus = GuardStatus.inProgress, +}) { + return PresubmitGuardResponse( + prNum: 1, + checkRunId: 1, + author: 'author', + guardStatus: guardStatus, + stages: [PresubmitGuardStage(name: 'stage', createdAt: 1234, jobs: jobs)], + ); +} + +/// Helper to create a valid JSON response string for testing [waitForTests]. +String _validResponseJson({ + required Map jobs, + String guardStatus = 'In Progress', +}) { + return jsonEncode({ + 'pr_num': 1, + 'check_run_id': 1, + 'author': 'author', + 'guard_status': guardStatus, + 'stages': [ + {'name': 'stage', 'created_at': 1234, 'jobs': jobs}, + ], + }); +} + void main() { group('evaluateTests', () { test('succeeds when all required tests succeed', () { - final json = { - 'stages': [ - { - 'jobs': { - 'Linux windows_host_engine': 'Succeeded', - 'Mac mac_ios_engine': 'Neutral', - 'Linux linux_fuchsia': 'Skipped', - }, - }, - ], - }; + final response = _createResponse( + jobs: { + 'Linux windows_host_engine': TaskStatus.succeeded, + 'Mac mac_ios_engine': TaskStatus.neutral, + 'Linux linux_fuchsia': TaskStatus.skipped, + }, + ); final requiredTests = [ 'Linux windows_host_engine', 'Mac mac_ios_engine', 'Linux linux_fuchsia', ]; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isTrue); expect(result.anyFailed, isFalse); @@ -58,18 +91,17 @@ void main() { }); test('fails when any required test fails', () { - final json = { - 'stages': [ - { - 'jobs': { - 'Linux windows_host_engine': 'Succeeded', - 'Mac mac_ios_engine': 'Failed', - }, - }, - ], - }; + final response = _createResponse( + jobs: { + 'Linux windows_host_engine': TaskStatus.succeeded, + 'Mac mac_ios_engine': TaskStatus.failed, + }, + ); final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isFalse); expect(result.anyFailed, isTrue); @@ -77,18 +109,17 @@ void main() { }); test('is pending when some tests are in progress or waiting', () { - final json = { - 'stages': [ - { - 'jobs': { - 'Linux windows_host_engine': 'Succeeded', - 'Mac mac_ios_engine': 'In Progress', - }, - }, - ], - }; + final response = _createResponse( + jobs: { + 'Linux windows_host_engine': TaskStatus.succeeded, + 'Mac mac_ios_engine': TaskStatus.inProgress, + }, + ); final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isFalse); expect(result.anyFailed, isFalse); @@ -96,15 +127,14 @@ void main() { }); test('treats missing required tests as waiting', () { - final json = { - 'stages': [ - { - 'jobs': {'Linux windows_host_engine': 'Succeeded'}, - }, - ], - }; + final response = _createResponse( + jobs: {'Linux windows_host_engine': TaskStatus.succeeded}, + ); final requiredTests = ['Linux windows_host_engine', 'Mac mac_ios_engine']; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isFalse); expect(result.anyFailed, isFalse); @@ -113,56 +143,46 @@ void main() { }); test('is case-insensitive and trims whitespace on job name matching', () { - final json = { - 'stages': [ - { - 'jobs': {' Linux windows_host_engine ': 'Succeeded'}, - }, - ], - }; + final response = _createResponse( + jobs: {' Linux windows_host_engine ': TaskStatus.succeeded}, + ); final requiredTests = ['linux windows_host_engine']; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isTrue); expect(result.anyFailed, isFalse); }); - test('correctly parses various status string variations', () { + test('correctly evaluates all status enum values', () { final statusMap = { - 'success': TaskStatus.succeeded, - 'succeeded': TaskStatus.succeeded, - 'neutral': TaskStatus.neutral, - 'skipped': TaskStatus.skipped, - 'failed': TaskStatus.failed, - 'infra_failure': TaskStatus.infraFailure, - 'infrafailure': TaskStatus.infraFailure, - 'cancelled': TaskStatus.cancelled, - 'canceled': TaskStatus.cancelled, - 'inprogress': TaskStatus.inProgress, - 'in_progress': TaskStatus.inProgress, - 'running': TaskStatus.inProgress, - 'new': TaskStatus.waitingForBackfill, - 'pending': TaskStatus.waitingForBackfill, - 'waiting': TaskStatus.waitingForBackfill, - 'queued': TaskStatus.waitingForBackfill, - 'scheduled': TaskStatus.waitingForBackfill, - 'some_weird_unsupported_status': TaskStatus.waitingForBackfill, + TaskStatus.succeeded: true, + TaskStatus.neutral: true, + TaskStatus.skipped: true, + TaskStatus.failed: false, + TaskStatus.infraFailure: false, + TaskStatus.cancelled: false, + TaskStatus.inProgress: false, + TaskStatus.waitingForBackfill: false, }; - for (final MapEntry(key: inputStatus, value: expectedStatus) - in statusMap.entries) { - final json = { - 'stages': [ - { - 'jobs': {'test_job': inputStatus}, - }, - ], - }; - final result = evaluateTests(json: json, requiredTests: ['test_job']); + for (final MapEntry(key: status, value: isSuccess) in statusMap.entries) { + final response = _createResponse(jobs: {'test_job': status}); + final result = evaluateTests( + response: response, + requiredTests: ['test_job'], + ); expect( result.summaries[0].status, - expectedStatus, - reason: 'Failed to parse "$inputStatus" to $expectedStatus', + status, + reason: 'Failed to verify status $status', + ); + expect( + result.allSucceeded, + isSuccess, + reason: 'Expected allSucceeded to be $isSuccess for status $status', ); } }); @@ -170,16 +190,15 @@ void main() { test( 'isGuardFailed causes anyFailed to be true even when requiredTests is provided and has no failed tests', () { - final json = { - 'guard_status': 'infra_failure', - 'stages': [ - { - 'jobs': {'Linux windows_host_engine': 'Succeeded'}, - }, - ], - }; + final response = _createResponse( + guardStatus: GuardStatus.failed, + jobs: {'Linux windows_host_engine': TaskStatus.succeeded}, + ); final requiredTests = ['Linux windows_host_engine']; - final result = evaluateTests(json: json, requiredTests: requiredTests); + final result = evaluateTests( + response: response, + requiredTests: requiredTests, + ); expect(result.allSucceeded, isTrue); expect(result.anyFailed, isTrue); @@ -190,17 +209,10 @@ void main() { group('waitForTests with fake_async', () { test('returns immediately on success on first check', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson(jobs: {'Linux windows_host_engine': 'Succeeded'}), + 200, + ); }); fakeAsync((async) { @@ -209,7 +221,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 10), client: mockClient, @@ -227,17 +239,10 @@ void main() { test('returns immediately on failure on first check', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Failed" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson(jobs: {'Linux windows_host_engine': 'Failed'}), + 200, + ); }); fakeAsync((async) { @@ -246,7 +251,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 10), client: mockClient, @@ -267,29 +272,19 @@ void main() { final mockClient = MockClient((request) async { callCount++; if (callCount == 1) { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "In progress" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'In Progress'}, + ), + 200, + ); } else { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); } }); @@ -299,7 +294,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -327,29 +322,17 @@ void main() { final mockClient = MockClient((request) async { callCount++; if (callCount == 1) { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "In progress" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'In Progress'}, + ), + 200, + ); } else { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Failed" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson(jobs: {'Linux windows_host_engine': 'Failed'}), + 200, + ); } }); @@ -359,7 +342,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -382,17 +365,12 @@ void main() { test('times out after configured duration', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "In progress" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'In Progress'}, + ), + 200, + ); }); fakeAsync((async) { @@ -401,7 +379,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -424,33 +402,27 @@ void main() { final mockClient = MockClient((request) async { callCount++; if (callCount == 1) { - return _stringResponse(''' - { - "guard_status": "In progress", - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded", - "Mac mac_ios_engine": "In progress" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + guardStatus: 'In Progress', + jobs: { + 'Linux windows_host_engine': 'Succeeded', + 'Mac mac_ios_engine': 'In Progress', + }, + ), + 200, + ); } else { - return _stringResponse(''' - { - "guard_status": "Succeeded", - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded", - "Mac mac_ios_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + guardStatus: 'Succeeded', + jobs: { + 'Linux windows_host_engine': 'Succeeded', + 'Mac mac_ios_engine': 'Succeeded', + }, + ), + 200, + ); } }); @@ -460,7 +432,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const [], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -485,19 +457,16 @@ void main() { test('fails immediately when any test fails in all-tests mode', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "guard_status": "Failed", - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Failed", - "Mac mac_ios_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + guardStatus: 'Failed', + jobs: { + 'Linux windows_host_engine': 'Failed', + 'Mac mac_ios_engine': 'Succeeded', + }, + ), + 200, + ); }); fakeAsync((async) { @@ -506,7 +475,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const [], waitInterval: const Duration(seconds: 10), client: mockClient, @@ -529,17 +498,12 @@ void main() { if (callCount == 1) { throw Exception('Transient network error'); } else { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); } }); @@ -550,7 +514,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -584,17 +548,12 @@ void main() { if (callCount == 1) { return _stringResponse('Gateway Timeout', 504); } else { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); } }); @@ -605,7 +564,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -641,17 +600,12 @@ void main() { if (callCount == 1) { return _stringResponse('not a json map', 200); } else { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); } }); @@ -662,7 +616,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 30), client: mockClient, @@ -693,19 +647,69 @@ void main() { }); }); + test( + 'continues polling when response is valid JSON but fails to parse into PresubmitGuardResponse', + () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse('{"invalid_field": true}', 200); + } else { + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + slug: RepositorySlug('flutter', 'flutter'), + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: valid JSON but fails to parse + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => l.contains( + 'Warning: Failed to parse JSON into PresubmitGuardResponse', + ), + ), + isTrue, + ); + + // Trigger second poll + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }, + ); + test('clamps waitInterval to at least 30 seconds and logs warning', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson(jobs: {'Linux windows_host_engine': 'Succeeded'}), + 200, + ); }); fakeAsync((async) { @@ -715,7 +719,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 10), client: mockClient, @@ -741,17 +745,10 @@ void main() { test('clamps waitInterval to at most 600 seconds and logs warning', () { final mockClient = MockClient((request) async { - return _stringResponse(''' - { - "stages": [ - { - "jobs": { - "Linux windows_host_engine": "Succeeded" - } - } - ] - } - ''', 200); + return _stringResponse( + _validResponseJson(jobs: {'Linux windows_host_engine': 'Succeeded'}), + 200, + ); }); fakeAsync((async) { @@ -761,7 +758,7 @@ void main() { waitForTests( sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', - repo: 'flutter', + slug: RepositorySlug('flutter', 'flutter'), requiredTests: const ['Linux windows_host_engine'], waitInterval: const Duration(seconds: 1000), client: mockClient, @@ -784,5 +781,148 @@ void main() { ); }); }); + + test('fails immediately on non-transient 4xx client errors (e.g. 404)', () { + final mockClient = MockClient((request) async { + return _stringResponse('Not Found', 404); + }); + + fakeAsync((async) { + var completed = false; + var successResult = true; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + slug: RepositorySlug('flutter', 'flutter'), + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + async.elapse(const Duration(milliseconds: 10)); + expect(completed, isTrue); + expect(successResult, isFalse); + expect( + logs.any( + (l) => l.contains('Error: Non-transient 4xx error from Cocoon API'), + ), + isTrue, + ); + }); + }); + + test('retries on transient 429 Too Many Requests errors', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse('Too Many Requests', 429); + } else { + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + slug: RepositorySlug('flutter', 'flutter'), + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: 429 logged as warning, sleep + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => l.contains('Warning: Cocoon API returned status code 429'), + ), + isTrue, + ); + + // Second poll: Succeeds + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + + test('retries when request hangs and triggers 15-second timeout', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + // Delay longer than 15 seconds to trigger timeout + await Future.delayed(const Duration(seconds: 20)); + return _stringResponse('late response', 200); + } else { + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + slug: RepositorySlug('flutter', 'flutter'), + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // Wait 16 seconds to elapse the 15-second timeout. + // It triggers TimeoutException, gets caught, logged, and starts delay. + async.elapse(const Duration(seconds: 16)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => + l.contains('Warning: Error calling Cocoon API') && + l.contains('TimeoutException'), + ), + isTrue, + ); + + // Advance to trigger the second poll (30s wait interval total elapsed since start is 46s) + async.elapse(const Duration(seconds: 30)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); }); } From 8e4c02c7261186999383625ebc9f234078175e61 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 13:58:09 -0700 Subject: [PATCH 5/7] feat(wait_for_tests): optimize JSON response parsing and fix test path resolution - Migrate response parsing in the polling loop to use the typed PresubmitGuardResponse model instead of manual Map parsing. - Refactor stage job collection construction using modern Dart collection spread operators. - Fix high CPU hammering bug by sleeping properly when receiving empty or malformed JSON. - Improve CLI test robustness by dynamically resolving wait_for_tests.dart path, allowing test suite to run successfully from either package or workspace root. - Ensure 0 warnings or errors on static analysis. --- .../wait_for_tests/lib/wait_for_tests.dart | 108 +++++++++--------- packages/wait_for_tests/test/bin_test.dart | 4 +- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/packages/wait_for_tests/lib/wait_for_tests.dart b/packages/wait_for_tests/lib/wait_for_tests.dart index 1a461b4729..9673783e9e 100644 --- a/packages/wait_for_tests/lib/wait_for_tests.dart +++ b/packages/wait_for_tests/lib/wait_for_tests.dart @@ -46,13 +46,9 @@ PollResult evaluateTests({ required PresubmitGuardResponse response, required List requiredTests, }) { - final allJobs = {}; - for (final stage in response.stages) { - for (final MapEntry(key: String key, value: TaskStatus value) - in stage.jobs.entries) { - allJobs[key] = value; - } - } + final allJobs = { + for (final stage in response.stages) ...stage.jobs, + }; // Pre-build a normalized lookup map to avoid O(M * N) iteration final normalizedJobs = { @@ -191,62 +187,64 @@ Future waitForTests({ json = {}; } - if (json.isNotEmpty) { - final PresubmitGuardResponse guardResponse; - try { - guardResponse = PresubmitGuardResponse.fromJson(json); - } catch (e) { - log( - 'Warning: Failed to parse JSON into PresubmitGuardResponse: $e', - ); - log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); - await Future.delayed(clampedWaitInterval); - continue; - } + if (json.isEmpty) { + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } + + final PresubmitGuardResponse guardResponse; + try { + guardResponse = PresubmitGuardResponse.fromJson(json); + } catch (e) { + log('Warning: Failed to parse JSON into PresubmitGuardResponse: $e'); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } + + final result = evaluateTests( + response: guardResponse, + requiredTests: requiredTests, + ); - final result = evaluateTests( - response: guardResponse, - requiredTests: requiredTests, + log('\n--- Current Test Status Summary ---'); + for (final summary in result.summaries) { + log( + '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', ); + } + if (requiredTests.isEmpty) { + log('Remaining tests: ${result.remainingCount}'); + log('Overall Guard Status: ${result.guardStatus}'); + } + log('-----------------------------------\n'); + + final bool isSuccess; + if (requiredTests.isEmpty) { + isSuccess = + result.allSucceeded && + result.guardStatus.toLowerCase() == 'succeeded'; + } else { + isSuccess = result.allSucceeded; + } - log('\n--- Current Test Status Summary ---'); - for (final summary in result.summaries) { + if (isSuccess) { + if (requiredTests.isEmpty) { log( - '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', + 'Success: Overall guard status succeeded and all tests completed successfully!', ); - } - if (requiredTests.isEmpty) { - log('Remaining tests: ${result.remainingCount}'); - log('Overall Guard Status: ${result.guardStatus}'); - } - log('-----------------------------------\n'); - - final bool isSuccess; - if (requiredTests.isEmpty) { - isSuccess = - result.allSucceeded && - result.guardStatus.toLowerCase() == 'succeeded'; } else { - isSuccess = result.allSucceeded; - } - - if (isSuccess) { - if (requiredTests.isEmpty) { - log( - 'Success: Overall guard status succeeded and all tests completed successfully!', - ); - } else { - log('Success: All required tests have completed successfully!'); - } - return true; + log('Success: All required tests have completed successfully!'); } + return true; + } - if (result.anyFailed) { - log( - 'Error: One or more tests have failed, or the overall guard status is failed.', - ); - return false; - } + if (result.anyFailed) { + log( + 'Error: One or more tests have failed, or the overall guard status is failed.', + ); + return false; } } } catch (e) { diff --git a/packages/wait_for_tests/test/bin_test.dart b/packages/wait_for_tests/test/bin_test.dart index c27eb898cb..cea923572d 100644 --- a/packages/wait_for_tests/test/bin_test.dart +++ b/packages/wait_for_tests/test/bin_test.dart @@ -6,7 +6,9 @@ import 'dart:io'; import 'package:test/test.dart'; void main() { - final binPath = 'bin/wait_for_tests.dart'; + final binPath = Directory('bin').existsSync() + ? 'bin/wait_for_tests.dart' + : 'packages/wait_for_tests/bin/wait_for_tests.dart'; test('prints usage when --help is passed', () async { final result = await Process.run('dart', [binPath, '--help']); From db9b096fe96acd6ef1a273c0cfaef55c6c08bfbf Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 15:16:05 -0700 Subject: [PATCH 6/7] reviewer comments + retries 404: retry because CICD might not have been created yet! while loop handling level shifting --- .../wait_for_tests/bin/wait_for_tests.dart | 4 + .../wait_for_tests/lib/wait_for_tests.dart | 167 +++++++++--------- 2 files changed, 90 insertions(+), 81 deletions(-) diff --git a/packages/wait_for_tests/bin/wait_for_tests.dart b/packages/wait_for_tests/bin/wait_for_tests.dart index bad73fced8..343df08bc3 100644 --- a/packages/wait_for_tests/bin/wait_for_tests.dart +++ b/packages/wait_for_tests/bin/wait_for_tests.dart @@ -162,6 +162,10 @@ void main(List arguments) async { stderr.writeln('Failure: Action completed with errors.'); exit(1); } + } catch (e, stackTrace) { + stderr.writeln('Error: An unexpected error occurred: $e'); + stderr.writeln(stackTrace); + exit(1); } finally { client.close(); } diff --git a/packages/wait_for_tests/lib/wait_for_tests.dart b/packages/wait_for_tests/lib/wait_for_tests.dart index 9673783e9e..1a8a4a67b4 100644 --- a/packages/wait_for_tests/lib/wait_for_tests.dart +++ b/packages/wait_for_tests/lib/wait_for_tests.dart @@ -152,106 +152,111 @@ Future waitForTests({ } log('Wait interval: ${clampedWaitInterval.inSeconds} seconds'); - while (true) { - if (timeout != null && clock.now().difference(startTime) > timeout) { - log('Error: Polling timed out after ${timeout.inMinutes} minutes.'); - return false; - } - + while (timeout == null || clock.now().difference(startTime) <= timeout) { + final http.Response response; try { - final response = await client + response = await client .get(url) .timeout(const Duration(seconds: 15)); - if (response.statusCode != 200) { - final failImmediately = switch (response.statusCode) { - 429 || 408 => false, - >= 400 && < 500 => true, - _ => false, - }; - - if (failImmediately) { - log( - 'Error: Non-transient 4xx error from Cocoon API. Status code ${response.statusCode}. Body: ${response.body}', - ); - return false; - } + } on Exception catch (e) { + log('Warning: Error calling Cocoon API: $e'); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } + if (response.statusCode != 200) { + final failImmediately = switch (response.statusCode) { + 429 || 408 => false, + >= 400 && < 500 => true, + _ => false, + }; + + if (failImmediately) { log( - 'Warning: Cocoon API returned status code ${response.statusCode}. Body: ${response.body}', + 'Error: Non-transient 4xx error from Cocoon API. Status code ${response.statusCode}. Body: ${response.body}', ); - } else { - Map json; - try { - json = jsonDecode(response.body) as Map; - } catch (e) { - log('Warning: Failed to parse Cocoon API response as JSON: $e'); - json = {}; - } + return false; + } + log( + 'Warning: Cocoon API returned status code ${response.statusCode}. Body: ${response.body}', + ); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } else { + Map json; + try { + json = jsonDecode(response.body) as Map; + } catch (e) { + log('Warning: Failed to parse Cocoon API response as JSON: $e'); + json = {}; + } - if (json.isEmpty) { - log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); - await Future.delayed(clampedWaitInterval); - continue; - } + if (json.isEmpty) { + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } - final PresubmitGuardResponse guardResponse; - try { - guardResponse = PresubmitGuardResponse.fromJson(json); - } catch (e) { - log('Warning: Failed to parse JSON into PresubmitGuardResponse: $e'); - log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); - await Future.delayed(clampedWaitInterval); - continue; - } + final PresubmitGuardResponse guardResponse; + try { + guardResponse = PresubmitGuardResponse.fromJson(json); + } catch (e) { + log('Warning: Failed to parse JSON into PresubmitGuardResponse: $e'); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } + + final result = evaluateTests( + response: guardResponse, + requiredTests: requiredTests, + ); - final result = evaluateTests( - response: guardResponse, - requiredTests: requiredTests, + log('\n--- Current Test Status Summary ---'); + for (final summary in result.summaries) { + log( + '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', ); + } + if (requiredTests.isEmpty) { + log('Remaining tests: ${result.remainingCount}'); + log('Overall Guard Status: ${result.guardStatus}'); + } + log('-----------------------------------\n'); - log('\n--- Current Test Status Summary ---'); - for (final summary in result.summaries) { - log( - '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', - ); - } - if (requiredTests.isEmpty) { - log('Remaining tests: ${result.remainingCount}'); - log('Overall Guard Status: ${result.guardStatus}'); - } - log('-----------------------------------\n'); + final bool isSuccess; + if (requiredTests.isEmpty) { + isSuccess = + result.allSucceeded && + result.guardStatus.toLowerCase() == 'succeeded'; + } else { + isSuccess = result.allSucceeded; + } - final bool isSuccess; + if (isSuccess) { if (requiredTests.isEmpty) { - isSuccess = - result.allSucceeded && - result.guardStatus.toLowerCase() == 'succeeded'; - } else { - isSuccess = result.allSucceeded; - } - - if (isSuccess) { - if (requiredTests.isEmpty) { - log( - 'Success: Overall guard status succeeded and all tests completed successfully!', - ); - } else { - log('Success: All required tests have completed successfully!'); - } - return true; - } - - if (result.anyFailed) { log( - 'Error: One or more tests have failed, or the overall guard status is failed.', + 'Success: Overall guard status succeeded and all tests completed successfully!', ); - return false; + } else { + log('Success: All required tests have completed successfully!'); } + return true; + } + + if (result.anyFailed) { + log( + 'Error: One or more tests have failed, or the overall guard status is failed.', + ); + return false; } - } catch (e) { - log('Warning: Error calling Cocoon API: $e'); } log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); await Future.delayed(clampedWaitInterval); } + + log('Error: Polling timed out after ${timeout.inMinutes} minutes.'); + return false; } From 705ae28e50c3dc194a304a0ca9138abacc524995 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 1 Jul 2026 15:48:05 -0700 Subject: [PATCH 7/7] reviewer comments --- .../wait_for_tests/lib/wait_for_tests.dart | 140 ++++++++---------- .../test/wait_for_tests_test.dart | 57 ++++++- 2 files changed, 117 insertions(+), 80 deletions(-) diff --git a/packages/wait_for_tests/lib/wait_for_tests.dart b/packages/wait_for_tests/lib/wait_for_tests.dart index 1a8a4a67b4..180c2173af 100644 --- a/packages/wait_for_tests/lib/wait_for_tests.dart +++ b/packages/wait_for_tests/lib/wait_for_tests.dart @@ -15,13 +15,8 @@ import 'package:http/http.dart' as http; class TestStatusSummary { final String name; final TaskStatus status; - final String originalStatusString; - const TestStatusSummary({ - required this.name, - required this.status, - required this.originalStatusString, - }); + const TestStatusSummary({required this.name, required this.status}); } /// The aggregated result of a polling check. @@ -75,14 +70,11 @@ PollResult evaluateTests({ final (matchedJobName, originalStatus) = lookup ?? (null, null); final TaskStatus status; - final String statusStr; if (originalStatus != null) { status = originalStatus; - statusStr = originalStatus.value; } else { status = TaskStatus.waitingForBackfill; - statusStr = 'Not yet scheduled'; } if (!status.isSuccess) { @@ -93,11 +85,7 @@ PollResult evaluateTests({ } summaries.add( - TestStatusSummary( - name: matchedJobName ?? trimmedName, - status: status, - originalStatusString: statusStr, - ), + TestStatusSummary(name: matchedJobName ?? trimmedName, status: status), ); } @@ -155,18 +143,21 @@ Future waitForTests({ while (timeout == null || clock.now().difference(startTime) <= timeout) { final http.Response response; try { - response = await client - .get(url) - .timeout(const Duration(seconds: 15)); + response = await client.get(url).timeout(const Duration(seconds: 15)); } on Exception catch (e) { log('Warning: Error calling Cocoon API: $e'); log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); await Future.delayed(clampedWaitInterval); continue; } + + // Handle error cases and either fail fast or sleep / retry. if (response.statusCode != 200) { final failImmediately = switch (response.statusCode) { - 429 || 408 => false, + // 404: we might not have created a check run yet. + // 408: request timed out on server. + // 429: try again + 404 || 429 || 408 => false, >= 400 && < 500 => true, _ => false, }; @@ -183,74 +174,69 @@ Future waitForTests({ log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); await Future.delayed(clampedWaitInterval); continue; - } else { - Map json; - try { - json = jsonDecode(response.body) as Map; - } catch (e) { - log('Warning: Failed to parse Cocoon API response as JSON: $e'); - json = {}; - } - - if (json.isEmpty) { - log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); - await Future.delayed(clampedWaitInterval); - continue; - } + } - final PresubmitGuardResponse guardResponse; - try { - guardResponse = PresubmitGuardResponse.fromJson(json); - } catch (e) { - log('Warning: Failed to parse JSON into PresubmitGuardResponse: $e'); - log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); - await Future.delayed(clampedWaitInterval); - continue; - } + // Success case. + Map json; + try { + json = jsonDecode(response.body) as Map; + } catch (e) { + log('Warning: Failed to parse Cocoon API response as JSON: $e'); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } - final result = evaluateTests( - response: guardResponse, - requiredTests: requiredTests, - ); + final PresubmitGuardResponse guardResponse; + try { + guardResponse = PresubmitGuardResponse.fromJson(json); + } catch (e) { + log('Warning: Failed to parse JSON into PresubmitGuardResponse: $e'); + log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); + await Future.delayed(clampedWaitInterval); + continue; + } - log('\n--- Current Test Status Summary ---'); - for (final summary in result.summaries) { - log( - '- ${summary.name}: ${summary.status.value} (${summary.originalStatusString})', - ); - } - if (requiredTests.isEmpty) { - log('Remaining tests: ${result.remainingCount}'); - log('Overall Guard Status: ${result.guardStatus}'); - } - log('-----------------------------------\n'); + final result = evaluateTests( + response: guardResponse, + requiredTests: requiredTests, + ); - final bool isSuccess; - if (requiredTests.isEmpty) { - isSuccess = - result.allSucceeded && - result.guardStatus.toLowerCase() == 'succeeded'; - } else { - isSuccess = result.allSucceeded; - } + log('\n--- Current Test Status Summary ---'); + for (final summary in result.summaries) { + log('- ${summary.name}: ${summary.status.value}'); + } + if (requiredTests.isEmpty) { + log('Remaining tests: ${result.remainingCount}'); + log('Overall Guard Status: ${result.guardStatus}'); + } + log('-----------------------------------\n'); - if (isSuccess) { - if (requiredTests.isEmpty) { - log( - 'Success: Overall guard status succeeded and all tests completed successfully!', - ); - } else { - log('Success: All required tests have completed successfully!'); - } - return true; - } + final bool isSuccess; + if (requiredTests.isEmpty) { + isSuccess = + result.allSucceeded && + result.guardStatus.toLowerCase() == 'succeeded'; + } else { + isSuccess = result.allSucceeded; + } - if (result.anyFailed) { + if (isSuccess) { + if (requiredTests.isEmpty) { log( - 'Error: One or more tests have failed, or the overall guard status is failed.', + 'Success: Overall guard status succeeded and all tests completed successfully!', ); - return false; + } else { + log('Success: All required tests have completed successfully!'); } + return true; + } + + if (result.anyFailed) { + log( + 'Error: One or more tests have failed, or the overall guard status is failed.', + ); + return false; } log('Sleeping for ${clampedWaitInterval.inSeconds} seconds...'); diff --git a/packages/wait_for_tests/test/wait_for_tests_test.dart b/packages/wait_for_tests/test/wait_for_tests_test.dart index 6e8aa824cf..66ec3d5b75 100644 --- a/packages/wait_for_tests/test/wait_for_tests_test.dart +++ b/packages/wait_for_tests/test/wait_for_tests_test.dart @@ -139,7 +139,6 @@ void main() { expect(result.allSucceeded, isFalse); expect(result.anyFailed, isFalse); expect(result.summaries[1].status, TaskStatus.waitingForBackfill); - expect(result.summaries[1].originalStatusString, 'Not yet scheduled'); }); test('is case-insensitive and trims whitespace on job name matching', () { @@ -782,9 +781,9 @@ void main() { }); }); - test('fails immediately on non-transient 4xx client errors (e.g. 404)', () { + test('fails immediately on non-transient 4xx client errors (e.g. 403)', () { final mockClient = MockClient((request) async { - return _stringResponse('Not Found', 404); + return _stringResponse('Neighborhood of the Beast', 466); }); fakeAsync((async) { @@ -816,6 +815,58 @@ void main() { }); }); + test('retries on 404 - CICD not yet started', () { + var callCount = 0; + final mockClient = MockClient((request) async { + callCount++; + if (callCount == 1) { + return _stringResponse('Not Found', 404); + } else { + return _stringResponse( + _validResponseJson( + jobs: {'Linux windows_host_engine': 'Succeeded'}, + ), + 200, + ); + } + }); + + fakeAsync((async) { + var completed = false; + var successResult = false; + final logs = []; + + waitForTests( + sha: 'd100ca3882520e04129ff2a5c09372ecec3b3860', + slug: RepositorySlug('flutter', 'flutter'), + requiredTests: const ['Linux windows_host_engine'], + waitInterval: const Duration(seconds: 30), + client: mockClient, + log: logs.add, + ).then((res) { + completed = true; + successResult = res; + }); + + // First poll: 404 logged as warning, sleep + async.elapse(const Duration(seconds: 2)); + expect(completed, isFalse); + expect(callCount, 1); + expect( + logs.any( + (l) => l.contains('Warning: Cocoon API returned status code 404'), + ), + isTrue, + ); + + // Second poll: Succeeds + async.elapse(const Duration(seconds: 29)); + expect(completed, isTrue); + expect(successResult, isTrue); + expect(callCount, 2); + }); + }); + test('retries on transient 429 Too Many Requests errors', () { var callCount = 0; final mockClient = MockClient((request) async {