-
Notifications
You must be signed in to change notification settings - Fork 331
Bootstrap the Flutter IJ with integrations tests #8487
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.
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
Conversation
e5d908a
to
720e408
Compare
720e408
to
2494c6c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
testSrc/integration/io/flutter/integrationTest/NewProjectUITest.kt
Outdated
Show resolved
Hide resolved
class MyProjectUITest { | ||
|
||
companion object { | ||
// Generate a unique folder name for the test project to avoid conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an odd place for the comment. It feels like it should be in initContext
. (Unless you want to say: "this will be updated to a unique folder name..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Thanks!
testSrc/integration/io/flutter/integrationTest/utils/NewProject.kt
Outdated
Show resolved
Hide resolved
The additional STARTER test framework is added, see https://github.com/JetBrains/intellij-ide-starter Thanks to others here who trailblazed, @jonathan1983, JetBrains/intellij-platform-plugin-template#537 and @helinx, flutter#8338 The change does not try to get the new tests working in the presubmit.
1d21155
to
d041a13
Compare
* found in the LICENSE file. | ||
*/ | ||
|
||
package io.flutter.integrationTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, missed this before. but the standard practice is to keep package names all lowercase. See:
https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you address this in a follow-up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very exciting!
return setupTestContext( | ||
"", IdeProductProvider.IC.copy( | ||
// TODO(team) should the version be fetched from some setting, i.e. System.getProperty("uiPlatformBuildVersion") | ||
buildNumber = "252.23892.409", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we inject from build.gradle.kts
and/or from the gradle.properties
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question.
|
||
@Test | ||
@Disabled("Need license configuration to test") | ||
fun newProjectWS() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to call this something like testNewProjectWS
? Maybe this is unnecessary with the annotation, but I find it hard to know where the test originates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
The additional STARTER test framework is added, see https://github.com/JetBrains/intellij-ide-starter
Thanks to others here who trailblazed, @jonathan1983, JetBrains/intellij-platform-plugin-template#537 and @helinx, #8338