Conversation
b00eac7 to
cf50809
Compare
| </includes> | ||
| <systemPropertyVariables> | ||
| <mavenPluginVersion>@pom.version@</mavenPluginVersion> | ||
| <runtimeVersion>${runtimeVersion}</runtimeVersion> |
There was a problem hiding this comment.
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.
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.
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.
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.
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.
You probalby copied from ci.maven..
Noting it's quite a bit different from:
I'll have to try on windows.
| } | ||
|
|
||
| /** | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
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.
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(); |
| // 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.
I"m confused if we're doing this as a workaround to the ci.maven issue or permanently?
| * Liberty JAR | ||
| */ | ||
| private void createUberJar() throws MojoExecutionException { | ||
| executeMojo(getPlugin(), goal("package"), |
There was a problem hiding this comment.
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: