From ad96c4f480b19b84fb822104239ce1b62ef4dfea Mon Sep 17 00:00:00 2001 From: Michael Seaton Date: Wed, 27 Nov 2024 10:45:53 -0500 Subject: [PATCH] SDK-347 - Follow-up with improved logging and test coverage --- .../openmrs/maven/plugins/AbstractSdkIT.java | 11 ++- .../org/openmrs/maven/plugins/DeployIT.java | 66 +++++++++++++++-- .../openmrs-distro-owa1.properties | 7 ++ .../openmrs-distro-owa2.properties | 7 ++ .../maven/plugins/utility/DefaultWizard.java | 74 +++++++++---------- 5 files changed, 112 insertions(+), 53 deletions(-) create mode 100644 integration-tests/src/test/resources/integration-test/openmrs-distro-owa1.properties create mode 100644 integration-tests/src/test/resources/integration-test/openmrs-distro-owa2.properties diff --git a/integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java b/integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java index fedff9a3..c4809f50 100644 --- a/integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java +++ b/integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java @@ -77,7 +77,7 @@ public abstract class AbstractSdkIT { protected File distroFile; protected Path testBaseDir; protected Path testResourceDir; - protected boolean preserveTestOutput; + protected boolean deleteTestArtifacts = false; public String resolveSdkArtifact() throws MojoExecutionException { Properties sdk = new Properties(); @@ -119,7 +119,7 @@ public void setup() throws Exception { testResourceDir = testBaseDir.resolve("test-resources"); testDirectoryPath = testBaseDir.resolve(getClass().getSimpleName() + "_" + nextCounter()); testDirectory = testDirectoryPath.toFile(); - preserveTestOutput = Boolean.parseBoolean(System.getProperty("preserveTestOutput")); + FileUtils.deleteQuietly(testDirectory); if (!testDirectory.mkdirs()) { throw new RuntimeException("Unable to create test directory: " + testDirectory); } @@ -128,15 +128,14 @@ public void setup() throws Exception { verifier.setAutoclean(false); addTaskParam("openMRSPath", testDirectory.getAbsolutePath()); distroFile = new File(testDirectory, DistroProperties.DISTRO_FILE_NAME); + deleteTestArtifacts = Boolean.parseBoolean(System.getProperty("deleteTestArtifacts")); } @After public void teardown() { verifier.resetStreams(); - if (preserveTestOutput) { - log.debug("Test output preserved: " + testDirectory.getName()); - } - else { + if (deleteTestArtifacts) { + log.debug("Removing test artifacts: " + testDirectory); FileUtils.deleteQuietly(testDirectory); } cleanAnswers(); diff --git a/integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java b/integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java index 2ac48786..64fe2746 100644 --- a/integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java +++ b/integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java @@ -13,8 +13,10 @@ import java.util.Properties; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItemInArray; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.openmrs.maven.plugins.SdkMatchers.hasNameStartingWith; import static org.openmrs.maven.plugins.SdkMatchers.hasUserOwa; import static org.openmrs.maven.plugins.SdkMatchers.serverHasVersion; @@ -148,8 +150,8 @@ public void deploy_shouldUpgradeDistroTo3_0_0() throws Exception { Server server = Server.loadServer(testServerId); assertThat(server, serverHasVersion("3.0.0")); - assertLogContains("+ Adds frontend spa"); - assertLogContains("+ Adds frontend configuration"); + assertLogContains("+ Assembles and builds new frontend spa"); + assertLogContains("+ Adds config package distro-emr-configuration 3.0.0"); } @Test @@ -165,7 +167,7 @@ public void deploy_shouldUpgradeDistroWithConfigPackage() throws Exception { assertFilePresent(testServerId, "configuration", "addresshierarchy", "addresshierarchy-core_demo.csv"); assertFilePresent(testServerId, "configuration", "conceptclasses", "conceptclasses-core_data.csv"); assertFilePresent(testServerId, "configuration", "encountertypes", "encountertypes_core-demo.csv"); - assertLogContains("+ Adds frontend configuration"); + assertLogContains("+ Adds config package distro-emr-configuration 3.0.0"); } @Test @@ -180,7 +182,7 @@ public void deploy_shouldUpgradeDistroWithContentPackage() throws Exception { assertFilePresent(testServerId, "configuration", "conceptclasses", "hiv", "conceptclasses.csv"); assertFilePresent(testServerId, "configuration", "conceptsources", "hiv", "conceptsources.csv"); assertFilePresent(testServerId, "configuration", "encountertypes", "hiv", "encountertypes.csv"); - assertLogContains("+ Adds frontend configuration"); + assertLogContains("+ Adds content package hiv 1.0.0"); } @Test @@ -197,7 +199,13 @@ public void deploy_shouldReplaceConfigurationAndContentIfChanged() throws Except assertFilePresent(testServerId, "configuration", "addresshierarchy", "addresshierarchy-core_demo.csv"); assertFilePresent(testServerId, "configuration", "conceptclasses", "conceptclasses-core_data.csv"); assertFilePresent(testServerId, "configuration", "encountertypes", "encountertypes_core-demo.csv"); - assertLogContains("+ Adds frontend configuration"); + assertLogContains("+ Adds config package distro-emr-configuration 3.0.0"); + + Server server = Server.loadServer(testDirectoryPath.resolve(testServerId)); + assertNotNull(server); + assertThat(server.getConfigArtifacts().size(), equalTo(1)); + assertThat(server.getConfigArtifacts().get(0).getArtifactId(), equalTo("distro-emr-configuration")); + assertThat(server.getContentArtifacts().size(), equalTo(0)); includeDistroPropertiesFile("openmrs-distro-content-package.properties"); addAnswer(testServerId); @@ -212,7 +220,14 @@ public void deploy_shouldReplaceConfigurationAndContentIfChanged() throws Except assertFilePresent(testServerId, "configuration", "conceptclasses", "hiv", "conceptclasses.csv"); assertFilePresent(testServerId, "configuration", "conceptsources", "hiv", "conceptsources.csv"); assertFilePresent(testServerId, "configuration", "encountertypes", "hiv", "encountertypes.csv"); - assertLogContains("^ Updates frontend configuration"); + assertLogContains("- Removes existing configuration"); + assertLogContains("+ Adds content package hiv 1.0.0"); + + server = Server.loadServer(testDirectoryPath.resolve(testServerId)); + assertNotNull(server); + assertThat(server.getConfigArtifacts().size(), equalTo(0)); + assertThat(server.getContentArtifacts().size(), equalTo(1)); + assertThat(server.getContentArtifacts().get(0).getArtifactId(), equalTo("hiv")); } @Test @@ -279,4 +294,43 @@ public void deploy_shouldInstallOwaAndOwaModule() throws Exception { Server server = Server.loadServer(testServerId); assertThat(server, hasUserOwa(new OwaId("conceptdictionary","1.0.0"))); } + + @Test + public void deploy_shouldDeployOwas() throws Exception { + testServerId = setupTestServer("referenceapplication:2.2"); + includeDistroPropertiesFile("openmrs-distro-owa1.properties"); + addAnswer(testServerId); + addAnswer("y"); + addAnswer("y"); + executeTask("deploy"); + assertSuccess(); + assertFilePresent(testServerId, "owa"); + assertFilePresent(testServerId, "owa", "addonmanager.owa"); + assertFilePresent(testServerId, "owa", "SystemAdministration.owa"); + assertFilePresent(testServerId, "owa", "orderentry.owa"); + assertLogContains("+ Adds owa addonmanager 1.0.0"); + assertLogContains("+ Adds owa sysadmin 1.2.0"); + assertLogContains("+ Adds owa orderentry 1.2.4"); + + Server server = Server.loadServer(testDirectoryPath.resolve(testServerId)); + assertNotNull(server); + assertThat(server.getOwaArtifacts().size(), equalTo(3)); + + // Re-deploy with a distro properties that includes additions, upgrades, downgrades, and removals + includeDistroPropertiesFile("openmrs-distro-owa2.properties"); + executeTask("deploy"); + assertSuccess(); + assertFilePresent(testServerId, "owa"); + assertFilePresent(testServerId, "owa", "addonmanager.owa"); + assertFilePresent(testServerId, "owa", "SystemAdministration.owa"); + assertFileNotPresent(testServerId, "owa", "orderentry.owa"); + assertFilePresent(testServerId, "owa", "conceptdictionary.owa"); + assertLogContains("^ Updates owa addonmanager 1.0.0 to 1.1.0"); + assertLogContains("v Downgrades owa sysadmin 1.2.0 to 1.1.0"); + assertLogContains("+ Adds owa conceptdictionary 1.0.0"); + assertLogContains("- Deletes owa orderentry 1.2.4"); + + server = Server.loadServer(testDirectoryPath.resolve(testServerId)); + assertThat(server.getOwaArtifacts().size(), equalTo(3)); + } } diff --git a/integration-tests/src/test/resources/integration-test/openmrs-distro-owa1.properties b/integration-tests/src/test/resources/integration-test/openmrs-distro-owa1.properties new file mode 100644 index 00000000..033dc8f7 --- /dev/null +++ b/integration-tests/src/test/resources/integration-test/openmrs-distro-owa1.properties @@ -0,0 +1,7 @@ +name=Content Package Example +version=1.0.0 +war.openmrs=2.6.9 +owa.addonmanager=1.0.0 +owa.sysadmin=1.2.0 +owa.orderentry=1.2.4 +db.h2.supported=true \ No newline at end of file diff --git a/integration-tests/src/test/resources/integration-test/openmrs-distro-owa2.properties b/integration-tests/src/test/resources/integration-test/openmrs-distro-owa2.properties new file mode 100644 index 00000000..a0a8db3e --- /dev/null +++ b/integration-tests/src/test/resources/integration-test/openmrs-distro-owa2.properties @@ -0,0 +1,7 @@ +name=Content Package Example +version=1.0.0 +war.openmrs=2.6.9 +owa.addonmanager=1.1.0 +owa.conceptdictionary=1.0.0 +owa.sysadmin=1.1.0 +db.h2.supported=true \ No newline at end of file diff --git a/sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DefaultWizard.java b/sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DefaultWizard.java index e00350ea..f085699a 100644 --- a/sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DefaultWizard.java +++ b/sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DefaultWizard.java @@ -91,13 +91,13 @@ public class DefaultWizard implements Wizard { static final String UPGRADE_CONFIRM_TMPL = "\nThe %s %s introduces the following changes:"; - static final String UPDATE_ARTIFACT_TMPL = "^ Updates %s %s to %s"; + static final String UPDATE_ARTIFACT_TMPL = "^ Updates %s %s %s to %s"; - static final String DOWNGRADE_ARTIFACT_TMPL = "v Downgrades %s %s to %s"; + static final String DOWNGRADE_ARTIFACT_TMPL = "v Downgrades %s %s %s to %s"; - static final String ADD_ARTIFACT_TMPL = "+ Adds %s %s"; + static final String ADD_ARTIFACT_TMPL = "+ Adds %s %s %s"; - static final String DELETE_ARTIFACT_TMPL = "- Deletes %s %s"; + static final String DELETE_ARTIFACT_TMPL = "- Deletes %s %s %s"; static final String NO_DIFFERENTIAL = "\nNo distribution changes found"; @@ -1037,31 +1037,38 @@ public boolean promptForConfirmDistroUpgrade(UpgradeDifferential upgradeDifferen writer.printf((UPGRADE_CONFIRM_TMPL) + "%n", distribution.getName(), distribution.getVersion()); if (warChanges.hasChanges()) { - writer.printf( - (warChanges.getDowngradedArtifacts().isEmpty() ? UPDATE_ARTIFACT_TMPL : DOWNGRADE_ARTIFACT_TMPL) + "%n", - warChanges.getNewArtifacts().get(0).getArtifactId(), - upgradeDifferential.getServer().getPlatformVersion(), - warChanges.getNewArtifacts().get(0).getVersion()); + String template = (warChanges.getDowngradedArtifacts().isEmpty() ? UPDATE_ARTIFACT_TMPL : DOWNGRADE_ARTIFACT_TMPL) + "%n"; + Artifact oldWar = warChanges.getOldArtifacts().get(0); + Artifact newWar = warChanges.getNewArtifacts().get(0); + writer.printf(template, "OpenMRS", "Core", oldWar.getVersion(), newWar.getVersion()); } - promptForArtifactChangesIfNecessary(moduleChanges); - promptForArtifactChangesIfNecessary(owaChanges); + promptForArtifactChangesIfNecessary("module", moduleChanges); + promptForArtifactChangesIfNecessary("owa", owaChanges); if (spaArtifactChanges.hasChanges() || spaBuildChanges.hasChanges()) { if (hasExistingFilesInDirectory(server, SDKConstants.OPENMRS_SERVER_FRONTEND)) { - writer.print("^ Updates frontend spa%n"); + writer.println("- Removes existing spa"); } - else { - writer.print("+ Adds frontend spa%n"); + if (spaBuildChanges.hasChanges()) { + writer.println("+ Assembles and builds new frontend spa"); + } + if (spaArtifactChanges.hasChanges()) { + for (Artifact a : spaArtifactChanges.getNewArtifacts()) { + writer.printf(ADD_ARTIFACT_TMPL + "%n", "spa", a.getArtifactId(), a.getVersion()); + } } } if (configChanges.hasChanges() || contentChanges.hasChanges()) { if (hasExistingFilesInDirectory(server, SDKConstants.OPENMRS_SERVER_CONFIGURATION)) { - writer.print("^ Updates frontend configuration%n"); + writer.println("- Removes existing configuration"); } - else { - writer.print("+ Adds frontend configuration%n"); + for (Artifact a : configChanges.getArtifactsToAdd()) { + writer.printf(ADD_ARTIFACT_TMPL + "%n", "config package", a.getArtifactId(), a.getVersion()); + } + for (Artifact a : contentChanges.getArtifactsToAdd()) { + writer.printf(ADD_ARTIFACT_TMPL + "%n", "content package", a.getArtifactId(), a.getVersion()); } } @@ -1077,34 +1084,19 @@ protected boolean hasExistingFilesInDirectory(Server server, String directory) { return false; } - protected void promptForArtifactChangesIfNecessary(UpgradeDifferential.ArtifactChanges artifactChanges) { + protected void promptForArtifactChangesIfNecessary(String type, UpgradeDifferential.ArtifactChanges artifactChanges) { if (artifactChanges.hasChanges()) { - for (Entry updateEntry : artifactChanges.getUpgradedArtifacts().entrySet()) { - if (!updateEntry.getKey().getVersion().equals(updateEntry.getValue().getVersion())) { - writer.printf((UPDATE_ARTIFACT_TMPL) + "%n", - updateEntry.getKey().getArtifactId(), - updateEntry.getKey().getVersion(), - updateEntry.getValue().getVersion()); - } + for (Entry e : artifactChanges.getUpgradedArtifacts().entrySet()) { + writer.printf(UPDATE_ARTIFACT_TMPL + "%n", type, e.getKey().getArtifactId(), e.getKey().getVersion(), e.getValue().getVersion()); } - for (Entry downgradeEntry : artifactChanges.getDowngradedArtifacts().entrySet()) { - if (!downgradeEntry.getKey().getVersion().equals(downgradeEntry.getValue().getVersion())) { - writer.printf((DOWNGRADE_ARTIFACT_TMPL) + "%n", - downgradeEntry.getKey().getArtifactId(), - downgradeEntry.getKey().getVersion(), - downgradeEntry.getValue().getVersion()); - } + for (Entry e : artifactChanges.getDowngradedArtifacts().entrySet()) { + writer.printf(DOWNGRADE_ARTIFACT_TMPL + "%n", type, e.getKey().getArtifactId(), e.getKey().getVersion(), e.getValue().getVersion()); } - for (Artifact addArtifact : artifactChanges.getAddedArtifacts()) { - writer.printf((ADD_ARTIFACT_TMPL) + "%n", - addArtifact.getArtifactId(), - addArtifact.getVersion()); + for (Artifact a : artifactChanges.getAddedArtifacts()) { + writer.printf(ADD_ARTIFACT_TMPL + "%n", type, a.getArtifactId(), a.getVersion()); } - - for (Artifact deleteArtifact : artifactChanges.getRemovedArtifacts()) { - writer.printf((DELETE_ARTIFACT_TMPL) + "%n", - deleteArtifact.getArtifactId(), - deleteArtifact.getVersion()); + for (Artifact a : artifactChanges.getRemovedArtifacts()) { + writer.printf(DELETE_ARTIFACT_TMPL + "%n", type, a.getArtifactId(), a.getVersion()); } } }