Skip to content
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.

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

[flutter_tools] shard out two integration tests we want to run on macOS arm64 #101769

Merged

Conversation

Copy link
Member

@christopherfujino christopherfujino commented Apr 12, 2022

Moved one test from integration.shard/bash_entrypoint_test.dart and the complete file of integration.shard/macos_content_validation_test.dart to a new shard, mac_arm.shard/. Also created a new builder in .ci.yaml, copying Mac tool_integration_tests 1_4, while deleting the goldctl dependency.

@flutter-dashboard flutter-dashboard bot added team tool labels Apr 12, 2022
Copy link
Member

@jmagman jmagman left a comment

I don't think mac_arm.shard makes sense, these tests need to run on x64 as well...
Should also include ios_content_validation_test for the simulator arch tests.

@christopherfujino
Copy link
Member Author

@christopherfujino christopherfujino commented Apr 12, 2022

I don't think mac_arm.shard makes sense, these tests need to run on x64 as well... Should also include ios_content_validation_test for the simulator arch tests.

host_cross_arch.shard?

Copy link
Contributor

@keyonghan keyonghan left a comment

Config LGTM, but we need to add the test ownership to pass the test.

@christopherfujino
Copy link
Member Author

@christopherfujino christopherfujino commented Apr 12, 2022

Config LGTM, but we need to add the test ownership to pass the test.

Should this be added as a comment near the end of TESTOWNERS? I'm not sure what's going on in that file.

@keyonghan
Copy link
Contributor

@keyonghan keyonghan commented Apr 12, 2022

Config LGTM, but we need to add the test ownership to pass the test.

Should this be added as a comment near the end of TESTOWNERS? I'm not sure what's going on in that file.

That's right.

@christopherfujino
Copy link
Member Author

@christopherfujino christopherfujino commented Apr 12, 2022

cc @Jasguerrero for context, regarding our 1:1 conversation, this is what it currently takes to create a new test shard.

.ci.yaml Outdated
{"dependency": "android_sdk", "version": "version:31v8"},
{"dependency": "chrome_and_driver", "version": "version:98.1"},
{"dependency": "open_jdk", "version": "11"},
Copy link
Member

@jmagman jmagman Apr 13, 2022

Choose a reason for hiding this comment

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

I know all the tasks have android_sdk and chrome_and_driver and open_jdk but I don't think they are needed for these tests. Can you try removing them and see if it passes? I've been meaning to audit all of them.

Copy link
Member Author

@christopherfujino christopherfujino Apr 13, 2022

Choose a reason for hiding this comment

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

SGTM, i already removed goldctl

Copy link
Member

@jmagman jmagman left a comment

LGTM, thanks Chris

@christopherfujino christopherfujino added the waiting for tree to go green label Apr 13, 2022
@fluttergithubbot fluttergithubbot merged commit fd5356f into flutter:master Apr 13, 2022
127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team tool waiting for tree to go green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants