-
Notifications
You must be signed in to change notification settings - Fork 13
add dev mode IT #401
base: master
Are you sure you want to change the base?
add dev mode IT #401
Conversation
b00eac7
to
cf50809
Compare
</includes> | ||
<systemPropertyVariables> | ||
<mavenPluginVersion>@pom.version@</mavenPluginVersion> | ||
<runtimeVersion>${runtimeVersion}</runtimeVersion> |
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.
Where is this runtimeVersion value coming from? (Our ITs are mostly not parameterized w/ runtime version)
<dependency> | ||
<groupId>io.openliberty.tools</groupId> | ||
<artifactId>liberty-maven-plugin</artifactId> | ||
<version>3.0.1</version> | ||
</dependency> |
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.
We really have LMP as a dependency? That seems surprising.. Is the version meaningful (e.g used to run the child POM in the test) or is this just like a "recent-enough version to compile against"?
</plugins> | ||
</build> | ||
|
||
<profiles> |
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.
I guess if it's probably easier to leave these in though you'd think we'll only use 'ol'
<dependency> | ||
<groupId>org.microshed.boost.boms</groupId> | ||
<artifactId>mp14-bom</artifactId> | ||
<version>RUNTIME_VERSION</version> |
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.
So that @pom.version@ thing doesn't work here? (I still haven't found the doc for that mechanism)
private static void startDevMode(String devModeParams) | ||
throws IOException, InterruptedException, FileNotFoundException { | ||
// run dev mode on project | ||
StringBuilder command = new StringBuilder("mvn io.openliberty.tools:liberty-maven-plugin:3.0.1:dev"); |
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.
Seems like we should extract the version: "3.0.1" into a property somehow that could be set in the POM and potentially the invoker plugin.
String os = System.getProperty("os.name"); | ||
if (os != null && os.toLowerCase().startsWith("windows")) { | ||
builder.command("CMD", "/C", processCommand); | ||
} else { | ||
builder.command("bash", "-c", processCommand); | ||
} |
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.
You probalby copied from ci.maven..
Noting it's quite a bit different from:
boost/boost-maven/boost-maven-plugin/src/it/test-dynamic-update/src/test/java/it/EndpointIT.java
Line 104 in bc74679
private void mavenCompile() throws IOException { |
I'll have to try on windows.
@@ -172,8 +177,13 @@ private void generateServerConfig(List<AbstractBoosterConfig> boosterConfigs) th | |||
} | |||
|
|||
/** | |||
<<<<<<< HEAD |
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.
merge conflict (though just in comment)
import org.junit.BeforeClass; | ||
import org.junit.Test; | ||
|
||
public class DevHotTestingTest extends BaseDevTest { |
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.
Do you think there's value in testing the "hotTests" path in boost specifically? If not maybe we could simplify by collapsing this into one class and reducing one moving part.
} | ||
|
||
public LibertyRuntime(RuntimeParams runtimeParams) { | ||
this.log = BoostLogger.getSystemStreamLogger(); |
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.
formatting
// the server is already running. If we do, the app will be deployed to | ||
// dropins | ||
// which is not what we want. | ||
if (!isServerRunning()) { |
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.
I"m confused if we're doing this as a workaround to the ci.maven issue or permanently?
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.
Comments attached.
* Invoke the liberty-maven-plugin to package the server into a runnable Liberty | ||
* JAR | ||
* Invoke the liberty-maven-plugin to package the server into a runnable | ||
* Liberty JAR | ||
*/ | ||
private void createUberJar() throws MojoExecutionException { | ||
executeMojo(getPlugin(), goal("package"), |
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.
Should we skip creating the runnable JAR if we're in dev mode? Is a running server a good proxy/test for this condition?
Notes: