Skip to content

Test the Android embedding v2 #42708

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
blasten merged 3 commits into from
Oct 17, 2019
Merged

Test the Android embedding v2 #42708

blasten merged 3 commits into from
Oct 17, 2019

Conversation

@blasten
Copy link

@blasten blasten commented Oct 15, 2019 •
edited
Loading

Description

Enables tests of the Android embedding v2.

I also renamed the flag as --android-embedding-v2.

Related Issues

@fluttergithubbot fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Oct 15, 2019
@codecov
Copy link

codecov bot commented Oct 15, 2019 •
edited
Loading

Codecov Report

Merging #42708 into master will increase coverage by 0.17%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #42708      +/-   ##
==========================================
+ Coverage   61.03%   61.21%   +0.17%     
==========================================
  Files         194      194              
  Lines       18919    18919              
==========================================
+ Hits        11547    11581      +34     
+ Misses       7372     7338      -34
Flag Coverage Δ
#flutter_tool 61.21% <100%> (+0.17%) ⬆️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/features.dart 97.56% <100%> (ø) ⬆️
packages/flutter_tools/lib/src/project.dart 87.8% <100%> (ø) ⬆️
...ackages/flutter_tools/lib/src/commands/create.dart 72.63% <100%> (ø) ⬆️
packages/flutter_tools/lib/src/base/build.dart 59.66% <0%> (-15.97%) ⬇️
packages/flutter_tools/lib/src/run_hot.dart 68.25% <0%> (-2.81%) ⬇️
packages/flutter_tools/lib/src/version.dart 93.23% <0%> (-1.94%) ⬇️
packages/flutter_tools/lib/src/cache.dart 48.47% <0%> (-0.71%) ⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart 57.37% <0%> (-0.61%) ⬇️
packages/flutter_tools/lib/src/vmservice.dart 41.73% <0%> (+0.66%) ⬆️
packages/flutter_tools/lib/src/template.dart 95.23% <0%> (+1.58%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 430f4c1...13ad5cb. Read the comment docs.

jonahwilliams
jonahwilliams reviewed Oct 15, 2019
View reviewed changes
jonahwilliams
jonahwilliams reviewed Oct 15, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Rename seems fine to me. Do we have a test that is running that executes flutter create and attempts to run the resulting app with the new embedding?

@blasten
Copy link
Author

blasten commented Oct 15, 2019

@jonahwilliams the exact combination of flutter create + flutter run doesn't exist yet. We do run an app using the new embedding on a real device, but the code is checked in.

Most of tests added in this PR are testing the build. Once #42684 is checked in, then we can migrate/clone the example apps to test hot reload.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Oct 15, 2019

I'd be concerned that the changes to create are not tested enough. I think there might be a devicelab test that does something like that

@blasten blasten force-pushed the test_new_embedding branch 2 times, most recently from 59495a2 to 2a645e3 Compare October 15, 2019 23:45
@blasten
Copy link
Author

blasten commented Oct 15, 2019

discussed offline. This first PR focuses on migrated the presubmit tests.

@blasten blasten requested a review from jonahwilliams October 15, 2019 23:53
@blasten blasten force-pushed the test_new_embedding branch 4 times, most recently from 122f3ed to 18867cf Compare October 16, 2019 04:17
@blasten blasten force-pushed the test_new_embedding branch from 18867cf to 13ad5cb Compare October 16, 2019 20:34
jonahwilliams
jonahwilliams approved these changes Oct 16, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Approach looks LGTM

@blasten blasten merged commit 08c645b into flutter:master Oct 17, 2019
@blasten blasten deleted the test_new_embedding branch October 17, 2019 04:26
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@Inconnu08
Test the Android embedding v2 (flutter#42708)
b24d08e 
* Test the Android embedding v2

* Update integration tests

* Split into shards
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@amirh amirh Awaiting requested review from amirh

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Create add-to-app embedding integration test

4 participants

@blasten @jonahwilliams @googlebot @fluttergithubbot