Skip to content
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

Refactor the Bazel Test configuration to support running tests on a single file or a single test #2960

Merged
merged 27 commits into from Jan 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
84220ea
Work on the contributor
Nov 26, 2018
495dea6
Make the bazel test runner work with individual files
Dec 7, 2018
7ee2dbf
Clean up test fields and configuration form
Dec 8, 2018
f79556a
Remove bazel test launcher script from the configuration; no longer used
Dec 11, 2018
5c482e6
Context menus work
Dec 12, 2018
5880036
Clean up template.xml and configutils, add a fallback to the test fields
Dec 13, 2018
000b0c7
Remove some test references from the bazelTest code.
Dec 21, 2018
1cd0a63
Get the test config producer working and set up a test file for the t…
Dec 21, 2018
30cc41a
Merge remote-tracking branch 'upstream/master' into bazel-test
Dec 21, 2018
2c828ec
Add tests for the test config
Jan 7, 2019
2a5de05
Add a setting for the new bazel test runner
Jan 8, 2019
a393164
Simplify the settings so that when the tester script is not available…
Jan 9, 2019
bb09ce6
Make the config producer work correctly
Jan 9, 2019
7fbdfc7
Remove redundant word
Jan 11, 2019
8c88e29
Bazel serialization test
Jan 12, 2019
3ce5b53
Tests for serialization
Jan 12, 2019
6d14464
File independence for the test fields
Jan 12, 2019
a953921
Start refactoring test config utils to share common logic between reg…
Jan 16, 2019
a375ac2
Nullables and fields refactoring
Jan 16, 2019
03da37d
Remove unused directory config producer
Jan 16, 2019
93272cd
Simplify line marker contributors to have simpler dependencies
Jan 16, 2019
934cf3e
Extra whitespace
Jan 16, 2019
dc98dd6
Update settings so that a warning shows when the new test runner cant…
Jan 16, 2019
7abf1cf
Revert testrunner changes
Jan 16, 2019
84e6a67
Remove dead code
Jan 16, 2019
c223926
Adjust override for bazel test fields setting
Jan 17, 2019
29aec71
Merge branch 'master' of ssh://github.com/flutter/flutter-intellij in…
Jan 17, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions resources/META-INF/plugin.xml
Expand Up @@ -720,7 +720,9 @@
<programRunner implementation="io.flutter.run.bazel.BazelRunner"/>

<configurationType implementation="io.flutter.run.bazelTest.FlutterBazelTestConfigurationType"/>
<runConfigurationProducer implementation="io.flutter.run.bazelTest.BazelTestConfigProducer"/>
Copy link
Member

Choose a reason for hiding this comment

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

This line will get deleted during the build unless it is added to the template. This may be the cause of the travis failure.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, you'll want to contribute the change to resources/META-INF/plugin_template.xml, and then run ./bin/plugin generate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<programRunner implementation="io.flutter.run.bazelTest.BazelTestRunner"/>
<runLineMarkerContributor language="Dart" implementationClass="io.flutter.run.bazelTest.FlutterBazelTestLineMarkerContributor"/>

<configurationType implementation="io.flutter.run.test.TestConfigType"/>
<runConfigurationProducer implementation="io.flutter.run.test.TestConfigProducer"/>
Expand Down
2 changes: 2 additions & 0 deletions resources/META-INF/plugin_template.xml
Expand Up @@ -196,7 +196,9 @@
<programRunner implementation="io.flutter.run.bazel.BazelRunner"/>

<configurationType implementation="io.flutter.run.bazelTest.FlutterBazelTestConfigurationType"/>
<runConfigurationProducer implementation="io.flutter.run.bazelTest.BazelTestConfigProducer"/>
<programRunner implementation="io.flutter.run.bazelTest.BazelTestRunner"/>
<runLineMarkerContributor language="Dart" implementationClass="io.flutter.run.bazelTest.FlutterBazelTestLineMarkerContributor"/>

<configurationType implementation="io.flutter.run.test.TestConfigType"/>
<runConfigurationProducer implementation="io.flutter.run.test.TestConfigProducer"/>
Expand Down
4 changes: 4 additions & 0 deletions src/io/flutter/FlutterBundle.properties
Expand Up @@ -70,6 +70,8 @@ flutter.run.bazel.noLaunchingScript=No launching script specified
flutter.run.bazel.launchingScriptNotFound=Launching script not found: {0}
flutter.run.bazel.noTargetSet=No Bazel target set
flutter.run.bazel.startWithSlashSlash=Bazel targets should start with '//'
flutter.run.bazel.newBazelTestRunnerUnavailable=The new Bazel test runner is not available. Only Bazel targets can be run with the old test runner
flutter.run.bazel.mustUseNewBazelTestRunner=The new Bazel test runner must be enabled to run tests from a specific file or test name. Enable this option from the Flutter plugin settings

flutter.perf.linter.statefulWidget.url=https://docs.flutter.io/flutter/widgets/StatefulWidget-class.html#performance-considerations
flutter.perf.linter.listViewLoad.url=https://docs.flutter.io/flutter/widgets/ListView-class.html
Expand Down Expand Up @@ -221,3 +223,5 @@ settings.flutter.version=Version:
settings.open.inspector.on.launch=Open Flutter Inspector view on app launch
settings.hot.reload.on.save=Perform hot reload on save
settings.disable.tracking.widget.creation=Disable tracking widget creation locations
settings.enable.bazel.test.runner=Enable new Bazel test runner
settings.enable.bazel.test.runner.mustSyncClientWarning=(Sync required)
47 changes: 43 additions & 4 deletions src/io/flutter/bazel/PluginConfig.java
Expand Up @@ -5,6 +5,7 @@
*/
package io.flutter.bazel;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
Expand All @@ -25,7 +26,7 @@
/**
* An in-memory snapshot of the flutter.json file from a Bazel workspace.
*/
class PluginConfig {
public class PluginConfig {
private final @NotNull Fields fields;

private PluginConfig(@NotNull Fields fields) {
Expand All @@ -47,6 +48,11 @@ String getLaunchScript() {
return fields.launchScript;
}

@Nullable
String getTestScript() {
return fields.testScript;
}

@Override
public boolean equals(Object obj) {
if (!(obj instanceof PluginConfig)) return false;
Expand Down Expand Up @@ -91,6 +97,22 @@ PluginConfig load(@NotNull VirtualFile file) {
return ApplicationManager.getApplication().runReadAction(readAction);
}

@VisibleForTesting
public static PluginConfig forTest(
@Nullable String daemonScript,
@Nullable String doctorScript,
@Nullable String launchScript,
@Nullable String testScript
) {
final Fields fields = new Fields(
daemonScript,
doctorScript,
launchScript,
testScript
);
return new PluginConfig(fields);
}

/**
* The JSON fields in a PluginConfig, as loaded from disk.
*/
Expand All @@ -109,26 +131,43 @@ private static class Fields {
private String doctorScript;

/**
*
* The script to run to start 'bazel'
*/
@SerializedName("launchScript")
private String launchScript;

/**
* The script to run to start 'flutter test'
*/
@SerializedName("testScript")
private String testScript;

Fields() {
}

/**
* Convenience constructor that takes all
*/
Fields(String daemonScript, String doctorScript, String launchScript, String testScript) {
this.daemonScript = daemonScript;
this.doctorScript = doctorScript;
this.launchScript = launchScript;
this.testScript = testScript;
}

@Override
public boolean equals(Object obj) {
if (!(obj instanceof Fields)) return false;
final Fields other = (Fields)obj;
return Objects.equal(daemonScript, other.daemonScript)
&& Objects.equal(doctorScript, other.doctorScript)
&& Objects.equal(launchScript, other.launchScript);
&& Objects.equal(launchScript, other.launchScript)
&& Objects.equal(testScript, other.testScript);
}

@Override
public int hashCode() {
return Objects.hashCode(daemonScript, doctorScript, launchScript);
return Objects.hashCode(daemonScript, doctorScript, launchScript, testScript);
}
}

Expand Down
85 changes: 50 additions & 35 deletions src/io/flutter/bazel/Workspace.java
Expand Up @@ -5,6 +5,7 @@
*/
package io.flutter.bazel;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -35,15 +36,18 @@ public class Workspace {
@Nullable private final PluginConfig config;
@Nullable private final String daemonScript;
@Nullable private final String doctorScript;
@Nullable private final String testScript;

private Workspace(@NotNull VirtualFile root,
@Nullable PluginConfig config,
@Nullable String daemonScript,
@Nullable String doctorScript) {
@Nullable String doctorScript,
@Nullable String testScript) {
this.root = root;
this.config = config;
this.daemonScript = daemonScript;
this.doctorScript = doctorScript;
this.testScript = testScript;
}

/**
Expand Down Expand Up @@ -119,6 +123,14 @@ public String getLaunchScript() {
return (config == null) ? null : config.getLaunchScript();
}

/**
* Returns the script that starts 'flutter test', or null if not configured.
*/
@Nullable
public String getTestScript() {
return testScript;
}

/**
* Returns true if the plugin config was loaded.
*/
Expand Down Expand Up @@ -171,45 +183,48 @@ public static Workspace load(@NotNull Project project) {
}
final PluginConfig config = configFile == null ? null : PluginConfig.load(configFile);

final String daemonScript;
if (config == null || config.getDaemonScript() == null) {
daemonScript = null;
}
else {
final String script = config.getDaemonScript();
final String readonlyScript = readonlyPath + "/" + script;
if (root.findFileByRelativePath(script) != null) {
daemonScript = script;
}
else if (root.findFileByRelativePath(readonlyScript) != null) {
daemonScript = readonlyScript;
}
else {
daemonScript = null;
}
}
final String daemonScript = config == null ? null : getScriptFromPath(root, readonlyPath, config.getDaemonScript());

final String doctorScript = config == null ? null : getScriptFromPath(root, readonlyPath, config.getDoctorScript());

final String testScript = config == null ? null : getScriptFromPath(root, readonlyPath, config.getTestScript());

return new Workspace(root, config, daemonScript, doctorScript, testScript);
}

@VisibleForTesting
public static Workspace forTest(VirtualFile workspaceRoot, PluginConfig pluginConfig) {
return new Workspace(
workspaceRoot,
pluginConfig,
pluginConfig.getDaemonScript(),
pluginConfig.getDoctorScript(),
pluginConfig.getTestScript()
);
}

final String doctorScript;
if (config == null || config.getDoctorScript() == null) {
doctorScript = null;
/**
* Attempts to find a script inside of the workspace.
* @param root the workspace root.
* @param readonlyPath the relative path to the readonly contents of the workspace.
* @param relativeScriptPath the relative path to the desired script inside of the workspace.
* @return the script's path relative to the workspace, or null if it was not found.
*/
private static String getScriptFromPath(@NotNull VirtualFile root, @NotNull String readonlyPath, @Nullable String relativeScriptPath) {
if (relativeScriptPath == null) {
return null;
}
else {
final String script = config.getDoctorScript();
final String readonlyScript = readonlyPath + "/" + script;
if (root.findFileByRelativePath(script) != null) {
doctorScript = script;
}
else if (root.findFileByRelativePath(readonlyScript) != null) {
doctorScript = readonlyScript;
}
else {
doctorScript = null;
}
final String readonlyScriptPath = readonlyPath + "/" + relativeScriptPath;
if (root.findFileByRelativePath(relativeScriptPath) != null) {
return relativeScriptPath;
}

return new Workspace(root, config, daemonScript, doctorScript);
if (root.findFileByRelativePath(readonlyScriptPath) != null) {
return readonlyScriptPath;
}
return null;
}


/**
* Returns the Bazel WORKSPACE file for a Project, or null if not using Bazel.
* <p>
Expand Down
9 changes: 6 additions & 3 deletions src/io/flutter/run/bazelTest/BazelTestConfig.java
Expand Up @@ -15,17 +15,20 @@
import com.intellij.openapi.util.WriteExternalException;
import com.intellij.util.xmlb.SkipDefaultValuesSerializationFilters;
import com.intellij.util.xmlb.XmlSerializer;
import io.flutter.bazel.Workspace;
import org.jdom.Element;
import org.jetbrains.annotations.NotNull;

/**
* The Bazel version of the {@link io.flutter.run.test.TestConfig}.
*/
public class BazelTestConfig extends LocatableConfigurationBase {
@NotNull private BazelTestFields fields = new BazelTestFields();
@NotNull private BazelTestFields fields;

BazelTestConfig(@NotNull final Project project, @NotNull final ConfigurationFactory factory, @NotNull final String name) {
super(project, factory, name);
final Workspace workspace = Workspace.load(project);
fields = new BazelTestFields(null, null, null);
}

@NotNull
Expand Down Expand Up @@ -72,12 +75,12 @@ RunConfiguration copyTemplateToNonTemplate(String name) {
@Override
public void writeExternal(@NotNull final Element element) throws WriteExternalException {
super.writeExternal(element);
XmlSerializer.serializeInto(fields, element, new SkipDefaultValuesSerializationFilters());
fields.writeTo(element);
}

@Override
public void readExternal(@NotNull final Element element) throws InvalidDataException {
super.readExternal(element);
XmlSerializer.deserializeInto(fields, element);
fields = BazelTestFields.readFrom(element);
}
}