Skip to content

Moves test ownership validation to flutter/flutter#188655

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
gaaclarke:test-ownership-move
Jun 29, 2026
Merged

Moves test ownership validation to flutter/flutter#188655
auto-submit[bot] merged 4 commits into
flutter:masterfrom
gaaclarke:test-ownership-move

Conversation

@gaaclarke

Copy link
Copy Markdown
Member

issue: #188640

This follows the pattern from #161249

This duplicates the logic of https://github.com/flutter/cocoon/blob/main/app_dart/bin/validate_task_ownership.dart

Note that the TESTOWNERS file format seems straightforward but it is actually weird where things that look like comments have semantic meaning.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the Linux test_ownership target from .ci.yaml and introduces a new Dart test, test_ownership_validation_test.dart, to validate that all CI targets in .ci.yaml have a defined owner in TESTOWNERS. Feedback on the changes highlights a compile-time error caused by attempting to instantiate the abstract class YamlMap directly, and suggests using RegExp(r'\s+') instead of split(' ') to robustly handle multiple consecutive spaces when parsing target names and owners.

Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
Comment thread dev/bots/test/test_ownership_validation_test.dart Outdated
@gaaclarke

Copy link
Copy Markdown
Member Author
Screenshot 2026-06-29 at 9 06 01 AM

I verified it was hooked up to ci, when introducing a failure the following tests failed.

@gaaclarke gaaclarke requested a review from jtmcdole June 29, 2026 16:49
@jtmcdole

Copy link
Copy Markdown
Member

Unpopular opinion, but I like:

  final isShard = switch ((recipe, tags?.contains('shard'), properties)) {
    ('flutter/flutter_drone', _, _) => true,
    (_, true, _) => true,
    (_, _, final p) when p['shard'] != null => true,
    _ => false,
  };

@gaaclarke

Copy link
Copy Markdown
Member Author

Oh yea, that's nice. I don't usually create new structures in order to send them to a match since I'm never sure if they are optimized away or not. It could be generating extra garbage.

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 29, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Jun 29, 2026
Merged via the queue into flutter:master with commit 4e78d1b Jun 29, 2026
172 of 173 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants