Rename e2e -> integration_test #2918
Conversation
dnfield
requested review from
amirh,
cyanglaz,
digiter,
LHLL,
matthew-carroll and
mklim
as code owners
|
The lint failure is because it's looking for a URL that doesn't exist yet, since this PR will be creating that URL. |
|
/cc @csells |
|
/cc @jiahaog as this will impact the internal roll/naming. |
| @@ -2,7 +2,7 @@ | |||
| // Use of this source code is governed by a BSD-style license that can be | |||
| // found in the LICENSE file. | |||
|
|
|||
| package dev.flutter.plugins.e2e; | |||
| package dev.flutter.plugins.integrationTest; | |||
There was a problem hiding this comment.
I believe package names should only be in all lowercase characters a-z characters where possible
https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html
| @@ -1,3 +1,3 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="dev.flutter.e2e"> | |||
| package="dev.flutter.integration_test"> | |||
There was a problem hiding this comment.
Should integration_test match the package of the Java sourcs?
There was a problem hiding this comment.
Ahh good catch, I'll make this match up.
|
I don't see an obvious way that the previously existing e2e package on pub would be linked to the new name from this PR, is that a concern? I think it would probably help to keep e2e as an empty package in the repo and publish a README update about the rename to pub, but I'm not really familiar with all the context here. |
|
That's a good idea. I was going to just locally modify e2e for that |
| import 'package:path/path.dart' as path; | ||
|
|
||
| /// This method remains for backword compatibility. | ||
| Future<void> main() => e2eDriver(); | ||
| Future<void> integration_main() => integrationDriver(); |
There was a problem hiding this comment.
If we are not calling it main anyway, why not just remove it? I was thinking if it's main it automatically can be used as a driver script. And keep it main can avoid breaking change to user's test.
|
And there's |
|
@CareF I've updated the READMe. I'm not calling it main so you can import it without an alias more easily and still use it. |
I get that, but I mean |
|
This is currently blocked by apparently unrelated timeouts on infra. Trying to open another PR to shard. |
|
Red CI is because this PR creates a directory the CI is checking for. Landing on red for that reason. If this goes red on post submit will revert. |
* e2e -> integration_test and associated version bumps
* e2e -> integration_test and associated version bumps
* e2e -> integration_test and associated version bumps
The e2e name is not entirely fitting for this plugin - some of our consumers expect e2e to implement larger multi-system tests exercising both client and server, which makes this package confusing. Adding the word test also should make this package more discoverable on pub for users looking for test packages.
WIP while I figure out which other plugin(s) need bumps to accomodate pubspec changes.