Skip to content

Conversation

stevemessick
Copy link
Member

@pq @jacob314 @devoncarew

With these changes one existing, rather simple UI integration test runs and passes.

I doubt that patching this in will make your tests run. The setup is non-trivial but I tried to document everything.

The new testData directory holds Flutter projects. These are unzipped into a temp directory as needed for running tests. I plan to extend the plugin tool to generate these during a build so the samples stay current with changes to the templates. While writing new tests you can just
cd testData/flutter_projects; flutter create ...
and use the project as-is. I prefer to zip the project up before checking it into the repo.

@devoncarew
Copy link
Member

Taking a spin through this now. I see that we're committing two test flutter projects (flutter-studio/testData/flutter_projects/simple_app/src.zip and the other).

For the devtools project, we committed the source version of the project, modulo the large binary files that can be re-created via flutter create. We may want to do the same thing here - it would likely make running and maintaining the test projects easier.

These could also be moved into a new top-level directory - fixtures/? We have some dart source at the top level of the project (in tool/). That may need to be moved into a lower level (tool/plugin/?) in order to make analysis happy.

We could also consider a large directory refactor of the repo, similar to way we did in devtools. Here's that directory structure now: https://github.com/flutter/devtools. Basically, fixtures/, and source/.

Potentially for us that would look like:

flutter-intellij/
flutter-studio/
docs/
fixtures/
infra/

Thoughts?

@devoncarew
Copy link
Member

The changes here rslgtm; in terms of checking in archives for the test fixtures, we should weight that against other options as well (checking in sources in a dir at the top level, or, refactoring the repo directory structure to better separate out sources and fixtures).

@stevemessick
Copy link
Member Author

I use the word "fixtures" as they do in the GUI testing framework, but I think you're using it differently. In their terminology it refers to a Java class used to drive a UI element, which may be anything from a Swing container to a single button or text field. Each fixture has methods to control individual UI items, like buttonFixture.click() and projectStepFixture.enterProjectName("BadName"). So, there's a fixture for each page of the New Project Wizard, and fixtures for the Project pane and the editor, and so forth. But they are used to write tests; they live under testSrc and only get compiled when building a test project.

These sample Flutter projects are test data, and need to be copied into the build directory during a test build, which IntelliJ does automatically. I was actually thinking that managing them could be done with an extension to the plugin generate tool that creates the projects and zips them up each time we do a release build. These zip files compress nicely, less than 150KB. In any case, I think we have to do pub get before running since the files get copied with no attempt (that I know of) to preserve time stamps.

Can you clarify "rslgtm"?

Rearranging the directory contents sounds like a great idea. Not sure how much work it will be. We'd want to preserve history as much as possible. And, for my own sanity, I'd prefer not to use "fixtures" as the name of the directory that contains sample projects for testing. We'd want to keep resources at top level, I think. The plugin tool would need updating to work with the new structure.

@devoncarew
Copy link
Member

devoncarew commented Feb 12, 2019

I use the word "fixtures" as they do in the GUI testing framework, but I think you're using it differently.

Totally possible :) I'm mostly using it to refer to the samples projects that we're using to support the UI tests. Possibly I should just call them sample projects...

Can you clarify "rslgtm"?

https://chromium.googlesource.com/chromiumos/docs/+/master/glossary.md - lgtm, but I didn't have quite enough context to do a full review.

These sample Flutter projects are test data, and need to be copied into the build directory during a test build, which IntelliJ does automatically.

I wonder if the tests could use them in-place? I guess if there're copied each time the tests are run it doesn't really matter.

I was actually thinking that managing them could be done with an extension to the plugin generate tool that creates the projects and zips them up each time we do a release build.

My guess is that they'd be a bit easier for developers to work with if they were available in source form.

Rearranging the directory contents sounds like a great idea.

Do you want to land this as is, and we can plan to refactor the repo after? Or, what do you think about just committing the sample apps to a new top-level directory (//samples/, //test-samples/, //flutter-samples/, ...)?

@stevemessick
Copy link
Member Author

I didn't have quite enough context to do a full review.

Yeah, that's an ongoing concern for these integration tests.

I wonder if the tests could use them in-place?

The integration tests run with CWD = studio repo root. The plugin sources are not even on the same disk (but I do at least have them on the same computer). I have not found a way to get from any known path in the test run config back to plugin sources. I could hard-code it but that wouldn't work well in general. Best I can tell, this is working as intended.

Do you want to land this as is, and we can plan to refactor the repo after?

I think it is best to keep things compartmentalized. Working on a project structure reorg after M33 seems like a good plan to me. I be happy to take that on -- let's discuss priorities.

@stevemessick stevemessick merged commit 7803ceb into master Feb 12, 2019
@stevemessick stevemessick deleted the ui-test-1-pass branch February 12, 2019 23:14
alexander-doroshko pushed a commit to alexander-doroshko/flutter-intellij that referenced this pull request Jan 24, 2020
* NewModuleTest passes

* cleanup

* merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants