Skip to content

Conversation

@ricardoamador
Copy link
Contributor

@ricardoamador ricardoamador commented Jun 29, 2023

Fix the test ownership test to compare the task_name with the dart file store in the TESTOWNERs file.

List which issues are fixed by this PR. You must list at least one issue.
Fixes flutter/flutter#125328

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@ricardoamador ricardoamador requested a review from keyonghan June 29, 2023 20:14
@ricardoamador ricardoamador changed the title Fix test ownership test to look for task_name if test is a devicelab test Fix test ownership test to use task_name if target is a devicelab target Jun 30, 2023
Comment on lines +44 to +48
String getTestNameFromTargetName(String targetName) {
// The builder names is in the format '<platform> <test name>'.
final List<String> words = targetName.split(' ');
return words.length < 2 ? words[0] : words[1];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we remove this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do believe that we still need this for the non devicelab tests as they still use the target name to split.

@ricardoamador ricardoamador force-pushed the testownership_check_125328 branch from 202dfa1 to 407d35c Compare June 30, 2023 17:21
Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM.

You need to wait for the flutter PR to land first to unblock the CQ here.

auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 30, 2023
Update a few of the tasks that are mislabeled as this will cause failures if that change is merged first.

This PR is needed before: flutter/cocoon#2898

*List which issues are fixed by this PR. You must list at least one issue.*
Part of #125328

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
@ricardoamador ricardoamador added the autosubmit Merge PR when tree becomes green via auto submit App. label Jun 30, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 30, 2023

auto label is removed for flutter/cocoon, pr: 2898, due to - This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Jun 30, 2023
@ricardoamador ricardoamador force-pushed the testownership_check_125328 branch from 6e0b1d5 to 7a1cd38 Compare June 30, 2023 19:36
@ricardoamador ricardoamador force-pushed the testownership_check_125328 branch from 7d16b29 to 01178b7 Compare June 30, 2023 19:46
@ricardoamador ricardoamador added the autosubmit Merge PR when tree becomes green via auto submit App. label Jun 30, 2023
@auto-submit auto-submit bot merged commit 4cd23a2 into flutter:main Jun 30, 2023
@ricardoamador ricardoamador deleted the testownership_check_125328 branch July 5, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_ownership check not working

2 participants