Skip to content

Update number of IPHONEOS_DEPLOYMENT_TARGET in plugin_test_ios - #91324

Merged
fluttergithubbot merged 2 commits into
flutter:masterfrom
jmagman:plugin-test-count
Oct 7, 2021
Merged

Update number of IPHONEOS_DEPLOYMENT_TARGET in plugin_test_ios#91324
fluttergithubbot merged 2 commits into
flutter:masterfrom
jmagman:plugin-test-count

Conversation

@jmagman

@jmagman jmagman commented Oct 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Change the integration test #72764 to explicitly check that the too-low plugin version number has been removed, instead of the expected build setting count. Which, as expected, was brittle...

Follow up to #91099
Will allow this test to be turned back on #88794

Task result:
{
  "success": true,
  "data": null,
  "detailFiles": [],
  "benchmarkScoreKeys": [],
  "reason": "success"
}

@jmagman jmagman added a: tests "flutter test", flutter_test, or one of our tests platform-ios iOS applications specifically labels Oct 5, 2021
@jmagman jmagman self-assigned this Oct 5, 2021
@flutter-dashboard flutter-dashboard Bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Oct 5, 2021
@google-cla google-cla Bot added the cla: yes label Oct 5, 2021
@jmagman jmagman mentioned this pull request Oct 5, 2021
Closed
@jmagman
jmagman requested a review from stuartmorgan-g October 6, 2021 00:11
stuartmorgan-g
stuartmorgan-g approved these changes Oct 6, 2021
View reviewed changes

@stuartmorgan-g stuartmorgan-g left a comment •
edited
Loading

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.

LGTM, but could you update the PR description to explain why the number is changing, so that it's clear to future people why this is correct (and not masking a regression as described in the comment just above this code)?

Comment thread dev/devicelab/lib/tasks/plugin_tests.dart Outdated
// See _reduceDarwinPluginMinimumVersion for details.
final int iosDeploymentTargetCount = 'IPHONEOS_DEPLOYMENT_TARGET'.allMatches(podsProjectContent).length;
if (iosDeploymentTargetCount != 9) {
if (iosDeploymentTargetCount != 12) {

@stuartmorgan-g stuartmorgan-g Oct 6, 2021

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.

Should the comment above that says "6 times" be updated too? (It already didn't match, so maybe there's a distinction between the two numbers that I'm missing.)

@jmagman jmagman Oct 6, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right, this is lame. Let me rework this check.

@jmagman jmagman Oct 7, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the description, test logic, and comment.

This may be a bit brittle

Thanks, past Jenn.

stuartmorgan-g
stuartmorgan-g approved these changes Oct 7, 2021
View reviewed changes

@stuartmorgan-g stuartmorgan-g 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.

LGTM

@fluttergithubbot
fluttergithubbot merged commit 4ef6fc1 into flutter:master Oct 7, 2021
@jmagman
jmagman deleted the plugin-test-count branch October 7, 2021 17:18
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021
@jmagman @clocksmith
Update number of IPHONEOS_DEPLOYMENT_TARGET in plugin_test_ios (flutt…
98ae927 
…er#91324)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

Assignees

@jmagman jmagman

Labels

a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmagman @stuartmorgan-g @fluttergithubbot