Skip to content

Conversation

@mravn-google
Copy link
Contributor

Adds integration test for platform channel communication.
Companion to flutter/engine#3631.

  • Moves existing DriverTest concept from the perf_test devicelab tasks to a new integration_test task file.
  • Redefines existing devicelab tests for the platform_channels sample to be integration_test tasks.
  • Adds new integration_test tasks for platform channel communication. The test itself is defined as a complete flutter app that can be controlled by flutter driver. It is placed in new folder dev/integration_tests
  • Various dartdoc fixes.
  • Some code simplifications.

@goderbauer
Copy link
Member

Can we also run this test on the Windows bot? I just fixed the platform channel communication when the app is deployed from Windows and would be great to keep that working :)

@yjbanov
Copy link
Contributor

yjbanov commented Apr 26, 2017

Can we also run this test on the Windows bot?

Let's do it, but if we're unsure that it will be immediately successful, let's mark it as flaky until it's stable.

Also, @mravn-google, @Hixie convinced me that we should just fix the dashboard to allow more tests. So feel free to ignore my suggestions about combining tests. Leaving it up to you.

@mravn-google
Copy link
Contributor Author

OK, so I did not combine the test. Instead I added a Windows version too. Taking up three more columns... I'd be happy to combine tests later, if we regret this.

@mravn-google mravn-google dismissed a stale review April 27, 2017 06:16

Per comment above.

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

LGTM

@mravn-google mravn-google merged commit 70ff50f into flutter:master Apr 27, 2017
Copy link
Member

@xster xster left a comment

Choose a reason for hiding this comment

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

This isn't a review. Just to confirm my understanding on the convention. (And I'm having some trouble with #9580)

From reading the flutter create template, it seems like the intent is to git ignore all the cocoapods artifacts but to check in/produce all the initial artifacts so that users can build and satisfy all the cocoapods' static scripts without actually installing cocoapods. So those artifacts will never change until the user starts using the plugin system via pubspec.yaml and installs cocoapods (unrelated but one potential danger is if the user deletes an artifact, things won't build anymore even though the deletion isn't going to be visible on git or revertable).

This project seems to be a bit different. The cocoapods are only partially checked in vs the flutter create template. So it won't build a first time until cocoapods is installed and 'install'ed. Though I suspect the project doesn't really conceptually depend on cocoapods?

Should we either do like the template and check in all the 'initial condition' artifacts (and perhaps not gitignore them?) or remove all actual dependencies on cocoapods and manually add them if one day this test does use cocoapods?

@@ -0,0 +1,4 @@
#include "Generated.xcconfig"
#include "Pods/Target Support Files/Pods-Runner/Pods-Runner.debug.xcconfig"
Copy link
Member

Choose a reason for hiding this comment

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

For instance, this file isn't checked in. So this project isn't buildable until the user runs pod install once though I suspect this project doesn't conceptually depend on cocoapods or the pub plugin system

3B3967161E833CAA004F5970 /* AppFrameworkInfo.plist in Resources */ = {isa = PBXBuildFile; fileRef = 3B3967151E833CAA004F5970 /* AppFrameworkInfo.plist */; };
3B80C3941E831B6300D905FE /* App.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3B80C3931E831B6300D905FE /* App.framework */; };
3B80C3951E831B6300D905FE /* App.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 3B80C3931E831B6300D905FE /* App.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
913134F849F6C8DEAC837F96 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C803412E9584DEAC0259A174 /* Pods_Runner.framework */; };
Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't checked in either

location = "group:Runner.xcodeproj">
</FileRef>
<FileRef
location = "group:Pods/Pods.xcodeproj">
Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't checked in either

@mravn-google
Copy link
Contributor Author

@xster I think it makes sense to remove all things cocoapods from the integration test. Thanks.

@mravn-google mravn-google deleted the update_dart_docs branch December 18, 2017 21:27
@jmagman jmagman mentioned this pull request Jun 2, 2020
10 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants