Skip to content

Make dev/customer_testing/tests.version test exempt #4588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
auto-submit merged 4 commits into from
Apr 24, 2025
Merged

Make dev/customer_testing/tests.version test exempt #4588

auto-submit merged 4 commits into from
Apr 24, 2025

Conversation

@gmackall
Copy link
Member

@gmackall gmackall commented Apr 24, 2025 •
edited
Loading

Makes the file test exempt. We could alternately count it as a test, but I think it is rare (and not a great practice) for someone to test the changes in their PR exclusively by modifying an element of the customer tests.

Motivated by flutter/flutter#167754

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.

@gmackall gmackall requested a review from matanlurey April 24, 2025 20:11
@gnprice
Copy link
Member

gnprice commented Apr 24, 2025

Thanks!

I think the ideal behavior might be to only make it test-exempt. That's because combining updates to tests.version with other changes is a dispreferred workflow for other reasons — see the thread at flutter/flutter#164818 (comment) (/cc @Piinks).

@gmackall
Copy link
Member Author

gmackall commented Apr 24, 2025

Thanks!

I think the ideal behavior might be to only make it test-exempt. That's because combining updates to tests.version with other changes is a dispreferred workflow for other reasons — see the thread at flutter/flutter#164818 (comment) (/cc @Piinks).

Fair enough, changed to suggested.

@gmackall gmackall changed the title Make dev/customer_testing/tests.version count as a test for exemption purposes Make dev/customer_testing/tests.version test exempt Apr 24, 2025
gnprice
gnprice approved these changes Apr 24, 2025
View reviewed changes
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jtmcdole jtmcdole self-requested a review April 24, 2025 23:19
@jtmcdole jtmcdole added the autosubmit Merge PR when tree becomes green via auto submit App. label Apr 24, 2025
@auto-submit auto-submit bot merged commit afe695f into flutter:main Apr 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gnprice gnprice gnprice approved these changes

@jtmcdole jtmcdole jtmcdole approved these changes

@matanlurey matanlurey Awaiting requested review from matanlurey

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gmackall @gnprice @jtmcdole