Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Make package:e2e a relative reference #2889

Merged
merged 12 commits into from
Jul 24, 2020
Merged

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Jul 21, 2020

Make sure that we pick up the package as it exists in this repo, rather than whatever happens to be published on pub.

This also meant updating some outdated/deprecated references (FlutterRunner -> FlutterTestRunner) in older usages.

/cc @CareF @digiter

@CareF
Copy link
Contributor

CareF commented Jul 21, 2020

I'm not sure if this works when those packages (not those files in example) are used in a user project, where the package is actually in somewhere like ~/.pub-cache/hosted/pub.dartlang.org/e2e-0.6.1/lib/e2e.dart? In which case, e.g. ~/.pub-cache/hosted/pub.dartlang.org/battery/pubspec.yaml's relative path ../../e2e may not be valid.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 21, 2020

Yes, this would mean you cannot run tests in your pub cache, but you shouldn't do that anyway

@dnfield dnfield requested a review from bkonyi as a code owner July 21, 2020 21:54
@dnfield dnfield force-pushed the e2e_relative branch 5 times, most recently from eaa4fef to 5db2455 Compare July 21, 2020 23:10
@ditman
Copy link
Member

ditman commented Jul 22, 2020

publishable is broken because of this issue: flutter/flutter#61747

@dnfield dnfield requested a review from digiter as a code owner July 22, 2020 22:21
@dnfield
Copy link
Contributor Author

dnfield commented Jul 22, 2020

@cyanglaz - I've re-enabled the test and fixed the issue with setSurfaceSize in the e2e binding in this PR.

/cc @CareF who might be interested in that.

@blasten
Copy link

blasten commented Jul 22, 2020

what about if someone tries to run the example app without downloading the repo though?

@dnfield
Copy link
Contributor Author

dnfield commented Jul 22, 2020

@blasten - how would they do that (short of trying to run it from their local .pub-cache, which I don't think we should worry about supporting)?

There's no straightforward way of getting just the example app for these projects. And anyone who is willing to do that should be able to just comment out the relative reference to e2e.

The bigger value here is that we'll actually know if a change to e2e is breaking (and/or if it's working as expected). We'll also have a clearer path to keeping the plugins up to date with the latest e2e stuff rather than reading several migration guides/changelogs to figure out what actually happened.

@blasten
Copy link

blasten commented Jul 22, 2020

The bigger value here is that we'll actually know if a change to e2e is breaking (and/or if it's working as expected). We'll also have a clearer path to keeping the plugins up to date with the latest e2e stuff rather than reading several migration guides/changelogs to figure out what actually happened.

Wouldn't that be the case for 3P plugins? e.g. how would the firebase plugins handle this?

@dnfield
Copy link
Contributor Author

dnfield commented Jul 23, 2020

Third party plugins cannot do this - if they want to test against master, they'd have to use a git reference. That's probably not desireable.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 23, 2020

To be clear - this is not because we want to recommend this method for other repositories, but because we want to catch regressions in e2e in this repository, and make sure that plugins in this repo are on the latest version of e2e.

@blasten
Copy link

blasten commented Jul 23, 2020

It sounds like the issue is test coverage of the e2e plugin itself. Is that correct? :)

@dnfield
Copy link
Contributor Author

dnfield commented Jul 23, 2020

It's both getting more coverage of e2e, as well as making sure that the plugin makes repository are using the latest version.

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Ship it!

@dnfield
Copy link
Contributor Author

dnfield commented Jul 24, 2020

Have to figure out why linux is timing out.

@ditman
Copy link
Member

ditman commented Jul 24, 2020

@dnfield maybe related to the latest push to the repo? I've pinged in the merged PR.

@dnfield dnfield merged commit 116b37d into flutter:master Jul 24, 2020
@dnfield dnfield deleted the e2e_relative branch July 24, 2020 23:00
jarrodcolburn pushed a commit to jarrodcolburn/plugins that referenced this pull request Aug 20, 2020
* Make package:e2e a relative reference
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
* Make package:e2e a relative reference
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
* Make package:e2e a relative reference
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants