From 9fa7762b39e94814d881a3d82528f694cb6c7a11 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 5 Jun 2024 12:06:30 -0400
Subject: [PATCH 01/11] AzureIntegrationTests share one billing profile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I first converted AzureIntegrationTest to JUnit5.
This was necessary in order to create @BeforeAll and @AfterAll methods to initialize a single Azure billing profile for use in all tests.

Expected benefits:

- If one test in the suite fails to delete its billing profile on tear-down, all other tests in the suite currently fail fast when trying to create datasets with errors like "Cannot reuse existing application deployment integrationapp from profile … with a different profile …".  This change should allow for more precise failure signaling, and increase the efficacy of retries.
- Serve as a model for sharing test initialization within a class, not currently used elsewhere in TDR.  Overall integration test runtime can take 2+ hours, and sharing set-up may help drive that down while incentivizing writing more granular tests when appropriate rather than overloading existing ones.
---
 .../terra/common/category/Integration.java    |  4 +-
 .../integration/AzureIntegrationTest.java     | 86 +++++++++++--------
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/src/test/java/bio/terra/common/category/Integration.java b/src/test/java/bio/terra/common/category/Integration.java
index 2b451d2f46..61c2911255 100644
--- a/src/test/java/bio/terra/common/category/Integration.java
+++ b/src/test/java/bio/terra/common/category/Integration.java
@@ -1,4 +1,6 @@
 package bio.terra.common.category;
 
 /** Integration test category. */
-public interface Integration {}
+public interface Integration {
+  String TAG = "bio.terra.common.category.Integration";
+}
diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
index cff325ae7b..48e1fac7e3 100644
--- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java
+++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
@@ -117,12 +117,15 @@
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.hamcrest.CoreMatchers;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Rule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -130,15 +133,16 @@
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.http.HttpStatus;
 import org.springframework.test.context.ActiveProfiles;
-import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.util.ResourceUtils;
 
-@RunWith(SpringRunner.class)
+@ExtendWith(SpringExtension.class)
 @SpringBootTest
 @ActiveProfiles({"google", "integrationtest"})
 @AutoConfigureMockMvc
-@Category(Integration.class)
-public class AzureIntegrationTest extends UsersBase {
+@Tag(Integration.TAG)
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class AzureIntegrationTest extends UsersBase {
   private static final Logger logger = LoggerFactory.getLogger(AzureIntegrationTest.class);
 
   private static final String omopDatasetName = "it_dataset_omop";
@@ -167,8 +171,8 @@ public class AzureIntegrationTest extends UsersBase {
   private GcsBlobIOTestUtility gcsBlobIOTestUtility;
   private Set<String> storageAccounts;
 
-  @Before
-  public void setup() throws Exception {
+  @BeforeAll
+  public void testClassSetup() throws Exception {
     super.setup(false);
     // Voldemort is required by this test since the application is deployed with his user authz'ed
     steward = steward("voldemort");
@@ -190,34 +194,44 @@ public void setup() throws Exception {
             null,
             retryOptions);
     gcsBlobIOTestUtility = new GcsBlobIOTestUtility(testConfig.getIngestbucket(), null);
+  }
+
+  @BeforeEach
+  public void testSetup() {
     snapshotIds = new ArrayList<>();
     snapshotAccessRequestIds = new ArrayList<>();
     storageAccounts = new TreeSet<>();
   }
 
-  @After
-  public void teardown() throws Exception {
+  @AfterEach
+  public void testTeardown() throws Exception {
     if (releaseSnapshotId != null) {
       snapshotIds.add(releaseSnapshotId);
     }
-    logger.info(
-        "Teardown: trying to delete snapshots {}, dataset {}, billing profile {}",
-        snapshotIds,
-        datasetId,
-        profileId);
+    logger.info("Test teardown: trying to delete snapshots {}, dataset {}", snapshotIds, datasetId);
 
     dataRepoFixtures.resetConfig(steward);
 
     for (UUID snapshotAccessRequestId : snapshotAccessRequestIds) {
       samFixtures.deleteSnapshotAccessRequest(steward, snapshotAccessRequestId);
     }
-
     for (UUID snapshotId : snapshotIds) {
       dataRepoFixtures.deleteSnapshot(steward, snapshotId);
     }
     if (datasetId != null) {
       dataRepoFixtures.deleteDataset(steward, datasetId);
     }
+    for (String storageAccountName : storageAccounts) {
+      deleteCloudResources(storageAccountName);
+    }
+    if (dac != null) {
+      samFixtures.deleteGroup(steward, dac);
+    }
+  }
+
+  @AfterAll
+  public void testClassTeardown() throws Exception {
+    logger.info("Test class teardown: trying to delete billing profile {}", profileId);
     if (profileId != null) {
       // TODO - https://broadworkbench.atlassian.net/browse/DCJ-228
       // As we move towards running smoke and integration tests in BEEs, we would like to do so
@@ -231,19 +245,16 @@ public void teardown() throws Exception {
       // rather than needing an admin to do it.
       dataRepoFixtures.deleteProfileWithCloudResourceDelete(admin, profileId);
     }
-    if (storageAccounts != null) {
-      storageAccounts.forEach(this::deleteCloudResources);
+    if (azureBlobIOTestUtility != null) {
+      azureBlobIOTestUtility.teardown();
     }
-    azureBlobIOTestUtility.teardown();
-    gcsBlobIOTestUtility.teardown();
-
-    if (dac != null) {
-      samFixtures.deleteGroup(steward, dac);
+    if (gcsBlobIOTestUtility != null) {
+      gcsBlobIOTestUtility.teardown();
     }
   }
 
   @Test
-  public void datasetsHappyPath() throws Exception {
+  void datasetsHappyPath() throws Exception {
     // Note: this region should not be the same as the default region in the application deployment
     // (eastus by default)
     AzureRegion region = AzureRegion.SOUTH_CENTRAL_US;
@@ -451,7 +462,7 @@ private void populateOmopTable() throws Exception {
   }
 
   @Test
-  public void testSnapshotCreateFromRequest() throws Exception {
+  void testSnapshotCreateFromRequest() throws Exception {
     populateOmopTable();
     UUID snapshotRequestId = makeSnapshotAccessRequest().getId();
     SnapshotSummaryModel snapshotSummaryByRequest = makeSnapshotFromRequest(snapshotRequestId);
@@ -511,7 +522,7 @@ private SnapshotSummaryModel makeSnapshotFromRequest(UUID requestSnapshotId) thr
   }
 
   @Test
-  public void testSnapshotBuilder() throws Exception {
+  void testSnapshotBuilder() throws Exception {
     populateOmopTable();
 
     var concept1 =
@@ -620,7 +631,7 @@ private void testRollupCountsWithCriteriaAndExpected(
   }
 
   @Test
-  public void datasetIngestFileHappyPath() throws Exception {
+  void datasetIngestFileHappyPath() throws Exception {
     String blobName = "myBlob";
     long fileSize = MIB / 10;
     String sourceFileAzure = azureBlobIOTestUtility.uploadSourceFile(blobName, fileSize);
@@ -1470,7 +1481,7 @@ sourceFileAzure, getSourceStorageAccountPrimarySharedKey()))
   }
 
   @Test
-  public void testDatasetFileIngestLoadHistory() throws Exception {
+  void testDatasetFileIngestLoadHistory() throws Exception {
     String blobName = "myBlob";
     long fileSize = MIB / 10;
     String sourceFile = azureBlobIOTestUtility.uploadSourceFile(blobName, fileSize);
@@ -1586,7 +1597,7 @@ sourceFile, getSourceStorageAccountPrimarySharedKey()))
   }
 
   @Test
-  public void testDatasetFileRefValidation() throws Exception {
+  void testDatasetFileRefValidation() throws Exception {
     DatasetSummaryModel summaryModel =
         dataRepoFixtures.createDataset(
             steward, profileId, "dataset-ingest-azure-fileref.json", CloudPlatform.AZURE);
@@ -1716,7 +1727,7 @@ public void testDatasetFileRefValidation() throws Exception {
   }
 
   @Test
-  public void testRequiredColumnsIngest() throws Exception {
+  void testRequiredColumnsIngest() throws Exception {
     DatasetSummaryModel summaryModel =
         dataRepoFixtures.createDataset(
             steward,
@@ -1780,16 +1791,16 @@ public void testRequiredColumnsIngest() throws Exception {
   }
 
   @Test
-  public void testDatasetCombinedIngest() throws Exception {
+  void testDatasetCombinedIngest() throws Exception {
     testDatasetCombinedIngest(true);
   }
 
   @Test
-  public void testDatasetCombinedIngestFromApi() throws Exception {
+  void testDatasetCombinedIngestFromApi() throws Exception {
     testDatasetCombinedIngest(false);
   }
 
-  public void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception {
+  void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception {
     DatasetSummaryModel summaryModel =
         dataRepoFixtures.createDataset(
             steward, profileId, "dataset-ingest-combined-azure.json", CloudPlatform.AZURE);
@@ -1831,8 +1842,7 @@ public void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception {
     dataRepoFixtures.assertCombinedIngestCorrect(ingestResponse, steward);
   }
 
-  public void testMetadataArrayIngest(String arrayIngestTableName, Object records)
-      throws Exception {
+  void testMetadataArrayIngest(String arrayIngestTableName, Object records) throws Exception {
     IngestRequestModel arrayIngestRequest =
         new IngestRequestModel()
             .ignoreUnknownValues(false)

From 82963c240c20bb8630da7e4c46ea047536e146c7 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 5 Jun 2024 15:22:08 -0400
Subject: [PATCH 02/11] JUnit 5 supports Extensions, not Rules

AzureIntegrationTest had a TestJobWatcher included as a @Rule, which is no longer supported in JUnit 5.
Instead, we can extend the test class with new TestJobWatcherExtension, which is compatible with JUnit 5.
Also refactored two assertions which still used JUnit 4 methods.
---
 .../integration/AzureIntegrationTest.java     |  9 ++--
 .../bio/terra/integration/TestJobWatcher.java | 43 +++------------
 .../integration/TestJobWatcherExtension.java  | 20 +++++++
 .../integration/TestJobWatcherUtils.java      | 53 +++++++++++++++++++
 4 files changed, 82 insertions(+), 43 deletions(-)
 create mode 100644 src/test/java/bio/terra/integration/TestJobWatcherExtension.java
 create mode 100644 src/test/java/bio/terra/integration/TestJobWatcherUtils.java

diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
index 48e1fac7e3..8d75f14d7b 100644
--- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java
+++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
@@ -11,8 +11,7 @@
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.startsWith;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import bio.terra.app.model.AzureCloudResource;
@@ -117,7 +116,6 @@
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.hamcrest.CoreMatchers;
-import org.junit.Rule;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
@@ -136,7 +134,7 @@
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.util.ResourceUtils;
 
-@ExtendWith(SpringExtension.class)
+@ExtendWith({SpringExtension.class, TestJobWatcherExtension.class})
 @SpringBootTest
 @ActiveProfiles({"google", "integrationtest"})
 @AutoConfigureMockMvc
@@ -155,7 +153,6 @@ class AzureIntegrationTest extends UsersBase {
   @Autowired private TestConfiguration testConfig;
   @Autowired private AzureResourceConfiguration azureResourceConfiguration;
   @Autowired private JsonLoader jsonLoader;
-  @Rule @Autowired public TestJobWatcher testWatcher;
 
   private User steward;
   private User admin;
@@ -361,7 +358,7 @@ void datasetsHappyPath() throws Exception {
               return found;
             });
 
-    assertTrue("dataset was found in enumeration", metExpectation);
+    assertThat("dataset was found in enumeration", metExpectation);
 
     // This should fail since it currently has dataset storage account within
     assertThrows(AssertionError.class, () -> dataRepoFixtures.deleteProfile(steward, profileId));
diff --git a/src/test/java/bio/terra/integration/TestJobWatcher.java b/src/test/java/bio/terra/integration/TestJobWatcher.java
index e1e5a85a0d..c8e85fe4ba 100644
--- a/src/test/java/bio/terra/integration/TestJobWatcher.java
+++ b/src/test/java/bio/terra/integration/TestJobWatcher.java
@@ -1,31 +1,17 @@
 package bio.terra.integration;
 
 import bio.terra.common.configuration.TestConfiguration;
-import java.net.URLEncoder;
-import java.nio.charset.StandardCharsets;
-import java.time.Duration;
-import java.time.Instant;
 import org.junit.rules.TestWatcher;
 import org.junit.runner.Description;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
-import org.stringtemplate.v4.ST;
 
-// This class can be used in a rule for integration test to decorate failures to make it easier to
-// debug
+/**
+ * This can be used as a Rule for JUnit 4 integration tests. For JUnit 5 integration tests, see
+ * TestJobWatcherExtension instead. It decorates failures with a link to GCP logs to make them
+ * easier to debug.
+ */
 @Component
 public class TestJobWatcher extends TestWatcher {
-  private static final Logger logger = LoggerFactory.getLogger(TestJobWatcher.class);
-
-  private static final String QUERY_TEMPLATE =
-      "resource.type=\"k8s_container\"\n"
-          + "resource.labels.project_id=\"broad-jade-integration\"\n"
-          + "resource.labels.location=\"us-central1\"\n"
-          + "resource.labels.cluster_name=\"integration-master\"\n"
-          + "resource.labels.namespace_name=\"integration-<intNumber>\"\n"
-          + "labels.k8s-pod/component=\"integration-<intNumber>-jade-datarepo-api";
-
   private final TestConfiguration testConfig;
 
   public TestJobWatcher(TestConfiguration testConfig) {
@@ -34,23 +20,6 @@ public TestJobWatcher(TestConfiguration testConfig) {
 
   @Override
   protected void failed(final Throwable e, final Description description) {
-    if (testConfig.getIntegrationServerNumber() != null) {
-      logger.error("See server log info at: {}", getStackdriverUrl());
-    }
-  }
-
-  private String getStackdriverUrl() {
-    String query =
-        URLEncoder.encode(
-            new ST(QUERY_TEMPLATE)
-                .add("intNumber", testConfig.getIntegrationServerNumber())
-                .render(),
-            StandardCharsets.UTF_8);
-    return "https://console.cloud.google.com/logs/query;"
-        + query
-        + ";cursorTimestamp="
-        + Instant.now().minus(Duration.ofSeconds(30)).toString()
-        + "?project="
-        + testConfig.getGoogleProjectId();
+    TestJobWatcherUtils.emitLinkToGcpLogs(testConfig);
   }
 }
diff --git a/src/test/java/bio/terra/integration/TestJobWatcherExtension.java b/src/test/java/bio/terra/integration/TestJobWatcherExtension.java
new file mode 100644
index 0000000000..92bf288818
--- /dev/null
+++ b/src/test/java/bio/terra/integration/TestJobWatcherExtension.java
@@ -0,0 +1,20 @@
+package bio.terra.integration;
+
+import static org.springframework.test.context.junit.jupiter.SpringExtension.getApplicationContext;
+
+import bio.terra.common.configuration.TestConfiguration;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestWatcher;
+
+/**
+ * This is an Extension for JUnit 5 integration tests. For JUnit 4 integration tests, use
+ * TestJobWatcher as a Rule instead. It decorates failures with a link to GCP logs to make them
+ * easier to debug.
+ */
+class TestJobWatcherExtension implements TestWatcher {
+  @Override
+  public void testFailed(ExtensionContext context, Throwable cause) {
+    TestConfiguration testConfig = getApplicationContext(context).getBean(TestConfiguration.class);
+    TestJobWatcherUtils.emitLinkToGcpLogs(testConfig);
+  }
+}
diff --git a/src/test/java/bio/terra/integration/TestJobWatcherUtils.java b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java
new file mode 100644
index 0000000000..a10bfc5c1a
--- /dev/null
+++ b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java
@@ -0,0 +1,53 @@
+package bio.terra.integration;
+
+import bio.terra.common.configuration.TestConfiguration;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.time.Instant;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.stringtemplate.v4.ST;
+
+/**
+ * Utility methods for reuse between Rules (JUnit 4) and Extensions (JUnit 5) which aide in
+ * debugging tests.
+ */
+public class TestJobWatcherUtils {
+
+  private static final Logger logger = LoggerFactory.getLogger(TestJobWatcherExtension.class);
+
+  private static final String QUERY_TEMPLATE =
+      """
+          resource.type="k8s_container"
+          resource.labels.project_id="broad-jade-integration"
+          resource.labels.location="us-central1"
+          resource.labels.cluster_name="integration-master"
+          resource.labels.namespace_name="integration-<intNumber>"
+          labels.k8s-pod/component="integration-<intNumber>-jade-datarepo-api""";
+
+  /**
+   * If these tests are running on an integration test server, emit a link to their recent logs in
+   * the GCP log console.
+   */
+  public static void emitLinkToGcpLogs(TestConfiguration testConfig) {
+    if (testConfig.getIntegrationServerNumber() != null) {
+      logger.error("See server log info at: {}", getLinkToGcpLogs(testConfig));
+    }
+  }
+
+  private static String getLinkToGcpLogs(TestConfiguration testConfig) {
+    String query =
+        URLEncoder.encode(
+            new ST(QUERY_TEMPLATE)
+                .add("intNumber", testConfig.getIntegrationServerNumber())
+                .render(),
+            StandardCharsets.UTF_8);
+    return "https://console.cloud.google.com/logs/query;"
+        + query
+        + ";cursorTimestamp="
+        + Instant.now().minus(Duration.ofSeconds(30)).toString()
+        + "?project="
+        + testConfig.getGoogleProjectId();
+  }
+}

From 53412c7cce57b916e07f464908bb73ba2a2f0dad Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 5 Jun 2024 15:22:41 -0400
Subject: [PATCH 03/11] Revert before merge: only run AzureIntegrationTest
 suite in CI

To make debugging easier.
---
 .github/workflows/int-and-connected-test-run.yml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/int-and-connected-test-run.yml b/.github/workflows/int-and-connected-test-run.yml
index 24f0cbc5da..8c08e31eab 100644
--- a/.github/workflows/int-and-connected-test-run.yml
+++ b/.github/workflows/int-and-connected-test-run.yml
@@ -204,16 +204,18 @@ jobs:
         env:
           DESIRED_GITHASH: ${{ steps.configuration.outputs.git_hash }}
           DEPLOYMENT_TYPE: 'api'
-      - name: "Run Test Runner smoke tests via Gradle"
-        uses: broadinstitute/datarepo-actions/actions/main@0.73.0
-        with:
-          actions_subcommand: 'gradletestrunnersmoketest'
+      # DO NOT MERGE.
+      # - name: "Run Test Runner smoke tests via Gradle"
+      #  uses: broadinstitute/datarepo-actions/actions/main@0.73.0
+      #  with:
+      #    actions_subcommand: 'gradletestrunnersmoketest'
       - name: "Run integration tests via Gradle"
         uses: broadinstitute/datarepo-actions/actions/main@0.73.0
         with:
           actions_subcommand: 'gradleinttest'
           pgport: ${{ job.services.postgres.ports[5432] }}
           test_to_run: 'testIntegration'
+          test_filter: 'AzureIntegrationTest' ## DO NOT MERGE.
         env:
           AZURE_CREDENTIALS_APPLICATIONID: ${{ env.AZURE_CREDENTIALS_APPLICATIONID }}
           AZURE_CREDENTIALS_HOMETENANTID: ${{ env.AZURE_CREDENTIALS_HOMETENANTID }}

From 1e768441563d976072502af5828c8a5e4fef8a81 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 5 Jun 2024 15:50:44 -0400
Subject: [PATCH 04/11] Also skip unit tests for now (will not merge)

---
 .../workflows/int-and-connected-test-run.yml   | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/int-and-connected-test-run.yml b/.github/workflows/int-and-connected-test-run.yml
index 8c08e31eab..f76dd3e93e 100644
--- a/.github/workflows/int-and-connected-test-run.yml
+++ b/.github/workflows/int-and-connected-test-run.yml
@@ -77,15 +77,15 @@ jobs:
         with:
           path: ${{ env.CACHE_PATHS }}
           key: ${{ runner.os }}-build-unit }}
-      - name: "Run unit tests and sonar scan via Gradle"
-        uses: broadinstitute/datarepo-actions/actions/main@0.73.0
-        with:
-          actions_subcommand: 'gradleinttest'
-          pgport: ${{ job.services.postgres.ports[5432] }}
-          test_to_run: 'check'
-          role_id: ${{ secrets.ROLE_ID }}
-          secret_id: ${{ secrets.SECRET_ID }}
-          sonar_token: ${{ secrets.SONAR_TOKEN }}
+#      - name: "Run unit tests and sonar scan via Gradle"
+#        uses: broadinstitute/datarepo-actions/actions/main@0.73.0
+#        with:
+#          actions_subcommand: 'gradleinttest'
+#          pgport: ${{ job.services.postgres.ports[5432] }}
+#          test_to_run: 'check'
+#          role_id: ${{ secrets.ROLE_ID }}
+#          secret_id: ${{ secrets.SECRET_ID }}
+#          sonar_token: ${{ secrets.SONAR_TOKEN }}
   test_connected:
     name: "Run connected tests"
     outputs:

From cf19624323c9f90ba199bf01efc67f9093e74dee Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 5 Jun 2024 15:55:48 -0400
Subject: [PATCH 05/11] Move blob IO test util initialization back to
 @BeforeEach methods

To help triangulate source of flaky integration tests.
The valuable part of this work is sharing a billing profile among tests, so removing other possible sources of flakiness.
---
 .../integration/AzureIntegrationTest.java     | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
index 8d75f14d7b..64ad6c71f9 100644
--- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java
+++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
@@ -169,13 +169,17 @@ class AzureIntegrationTest extends UsersBase {
   private Set<String> storageAccounts;
 
   @BeforeAll
-  public void testClassSetup() throws Exception {
+  public void createSharedBillingProfile() throws Exception {
     super.setup(false);
     // Voldemort is required by this test since the application is deployed with his user authz'ed
     steward = steward("voldemort");
     admin = admin("hermione");
     dataRepoFixtures.resetConfig(steward);
     profileId = dataRepoFixtures.createAzureBillingProfile(steward).getId();
+  }
+
+  @BeforeEach
+  public void testSetup() {
     RequestRetryOptions retryOptions =
         new RequestRetryOptions(
             RetryPolicyType.EXPONENTIAL,
@@ -191,10 +195,6 @@ public void testClassSetup() throws Exception {
             null,
             retryOptions);
     gcsBlobIOTestUtility = new GcsBlobIOTestUtility(testConfig.getIngestbucket(), null);
-  }
-
-  @BeforeEach
-  public void testSetup() {
     snapshotIds = new ArrayList<>();
     snapshotAccessRequestIds = new ArrayList<>();
     storageAccounts = new TreeSet<>();
@@ -221,13 +221,19 @@ public void testTeardown() throws Exception {
     for (String storageAccountName : storageAccounts) {
       deleteCloudResources(storageAccountName);
     }
+    if (azureBlobIOTestUtility != null) {
+      azureBlobIOTestUtility.teardown();
+    }
+    if (gcsBlobIOTestUtility != null) {
+      gcsBlobIOTestUtility.teardown();
+    }
     if (dac != null) {
       samFixtures.deleteGroup(steward, dac);
     }
   }
 
   @AfterAll
-  public void testClassTeardown() throws Exception {
+  public void deleteSharedBillingProfile() throws Exception {
     logger.info("Test class teardown: trying to delete billing profile {}", profileId);
     if (profileId != null) {
       // TODO - https://broadworkbench.atlassian.net/browse/DCJ-228
@@ -242,12 +248,6 @@ public void testClassTeardown() throws Exception {
       // rather than needing an admin to do it.
       dataRepoFixtures.deleteProfileWithCloudResourceDelete(admin, profileId);
     }
-    if (azureBlobIOTestUtility != null) {
-      azureBlobIOTestUtility.teardown();
-    }
-    if (gcsBlobIOTestUtility != null) {
-      gcsBlobIOTestUtility.teardown();
-    }
   }
 
   @Test

From b19068761120e88e9b049202a5e055d3e5a37a15 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Mon, 10 Jun 2024 16:04:02 -0400
Subject: [PATCH 06/11] @pshapiro4broad PR feedback

- Lost a closing quote when converting to multiline string
- Typo in docstring
---
 .../java/bio/terra/integration/TestJobWatcherUtils.java  | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/test/java/bio/terra/integration/TestJobWatcherUtils.java b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java
index a10bfc5c1a..861e89518b 100644
--- a/src/test/java/bio/terra/integration/TestJobWatcherUtils.java
+++ b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java
@@ -10,12 +10,12 @@
 import org.stringtemplate.v4.ST;
 
 /**
- * Utility methods for reuse between Rules (JUnit 4) and Extensions (JUnit 5) which aide in
- * debugging tests.
+ * Utility methods for reuse between Rules (JUnit 4) and Extensions (JUnit 5) which aid in debugging
+ * tests.
  */
 public class TestJobWatcherUtils {
 
-  private static final Logger logger = LoggerFactory.getLogger(TestJobWatcherExtension.class);
+  private static final Logger logger = LoggerFactory.getLogger(TestJobWatcherUtils.class);
 
   private static final String QUERY_TEMPLATE =
       """
@@ -24,7 +24,8 @@ public class TestJobWatcherUtils {
           resource.labels.location="us-central1"
           resource.labels.cluster_name="integration-master"
           resource.labels.namespace_name="integration-<intNumber>"
-          labels.k8s-pod/component="integration-<intNumber>-jade-datarepo-api""";
+          labels.k8s-pod/component="integration-<intNumber>-jade-datarepo-api"
+          """;
 
   /**
    * If these tests are running on an integration test server, emit a link to their recent logs in

From 4860ebd9906ece162c8f415e36b7efc8aca6ad87 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Mon, 10 Jun 2024 16:18:46 -0400
Subject: [PATCH 07/11] Make UsersBase an injectable component

Autowire it into AzureIntegrationTest rather than having the test extend it.
Fix an instance where we mistakenly called UsersBase.steward() in a test where we should have used the steward user initialized in test setup.
---
 .../bio/terra/integration/AzureIntegrationTest.java   | 11 ++++++-----
 src/test/java/bio/terra/integration/UsersBase.java    |  2 ++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
index 64ad6c71f9..64c513778e 100644
--- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java
+++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
@@ -140,13 +140,14 @@
 @AutoConfigureMockMvc
 @Tag(Integration.TAG)
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
-class AzureIntegrationTest extends UsersBase {
+class AzureIntegrationTest {
   private static final Logger logger = LoggerFactory.getLogger(AzureIntegrationTest.class);
 
   private static final String omopDatasetName = "it_dataset_omop";
   private static final String omopDatasetDesc =
       "OMOP schema based on BigQuery schema from https://github.com/OHDSI/CommonDataModel/wiki with extra columns suffixed with _custom";
 
+  @Autowired private UsersBase usersBase;
   @Autowired private DataRepoFixtures dataRepoFixtures;
   @Autowired private SamFixtures samFixtures;
   @Autowired private AuthService authService;
@@ -170,10 +171,10 @@ class AzureIntegrationTest extends UsersBase {
 
   @BeforeAll
   public void createSharedBillingProfile() throws Exception {
-    super.setup(false);
+    usersBase.setup(false);
     // Voldemort is required by this test since the application is deployed with his user authz'ed
-    steward = steward("voldemort");
-    admin = admin("hermione");
+    steward = usersBase.steward("voldemort");
+    admin = usersBase.admin("hermione");
     dataRepoFixtures.resetConfig(steward);
     profileId = dataRepoFixtures.createAzureBillingProfile(steward).getId();
   }
@@ -883,7 +884,7 @@ sourceFileAzure, getSourceStorageAccountPrimarySharedKey()))
                     .addTables(
                         List.of(
                             DatasetFixtures.tableModel("new_table", List.of("new_table_column")))));
-    DatasetModel response = dataRepoFixtures.updateSchema(steward(), datasetId, updateModel);
+    DatasetModel response = dataRepoFixtures.updateSchema(steward, datasetId, updateModel);
     assertThat(
         "The new table is in the update response",
         response.getSchema().getTables().stream()
diff --git a/src/test/java/bio/terra/integration/UsersBase.java b/src/test/java/bio/terra/integration/UsersBase.java
index c7f59c5e1d..7566972f53 100644
--- a/src/test/java/bio/terra/integration/UsersBase.java
+++ b/src/test/java/bio/terra/integration/UsersBase.java
@@ -6,7 +6,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
 
+@Component
 public class UsersBase {
   private static final String ADMIN_ROLE = "admin";
   private static final String STEWARD_ROLE = "steward";

From 6acbeaff38deffc393611f7f9cde78b5e09571fa Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Mon, 10 Jun 2024 16:19:48 -0400
Subject: [PATCH 08/11] Fix typos in shell commands documented in
 render-configs.sh

---
 render-configs.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/render-configs.sh b/render-configs.sh
index 26aa965a3c..3780b2e4b5 100755
--- a/render-configs.sh
+++ b/render-configs.sh
@@ -10,13 +10,13 @@
 
 # If you're running Azure Integration Tests you should use the following settings:
 # ./render-configs.sh -a integration -r tools
-# Then, refresh your z-shell configuration (`source ~./zshrc`) (follow getting started doc to set env variables)
+# Then, refresh your z-shell configuration (`source ~/.zshrc`) (follow getting started doc to set env variables)
 # Alternatively, if you use the -i flag, it copies the environment variables to your clipboard and you can paste them into your Intellij test profile.
 # ./render-configs.sh -a integration -r tools -i
 
 # If you want a set up locally, you can use the following settings:
 # ./render-configs.sh -a dev -r dev
-# Then, refresh your z-shell configuration (`source ~./zshrc`)
+# Then, refresh your z-shell configuration (`source ~/.zshrc`)
 # ./gradlew bootRun
 
 AZURE_ENV=dev

From 6c1199f152311e9dd793879ae7dbf62bbf58a95a Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Mon, 10 Jun 2024 18:26:45 -0400
Subject: [PATCH 09/11] On Azure profile create, register association with
 deployed app

Previously, the deployed app <-> billing profile association is registered the first time a dataset is created off of the profile.
I believe this is producing a race condition / flaky integration tests using the same billing profile.

- Added new CreateProfileRegisterDeployedApplicationStep in ProfileCreateFlight
- Notably, added this after the profile metadata was saved and made available as a job response in the working map
- Expanded docstrings, unit testing of AzureApplicationDeploymentService.getOrRegisterApplicationDeployment
---
 .../profile/flight/ProfileMapKeys.java        |  1 +
 ...rofileRegisterDeployedApplicationStep.java | 57 ++++++++++++
 .../flight/create/ProfileCreateFlight.java    |  4 +
 .../AzureApplicationDeploymentService.java    | 18 ++--
 .../terra/flight/ProfileCreateFlightTest.java |  1 +
 ...AzureApplicationDeploymentServiceTest.java | 87 +++++++++++++++----
 6 files changed, 143 insertions(+), 25 deletions(-)
 create mode 100644 src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java

diff --git a/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java b/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java
index 6cf898a642..3ca5e967bb 100644
--- a/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java
+++ b/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java
@@ -8,6 +8,7 @@ public final class ProfileMapKeys {
       "profileApplicationDeploymentIdList";
   public static final String PROFILE_UNIQUE_STORAGE_ACCOUNT_RESOURCE_LIST =
       "profileUniqueStorageAccountResourceList";
+  public static final String PROFILE_APPLICATION_DEPLOYMENT = "profileApplicationDeployment";
 
   private ProfileMapKeys() {}
 }
diff --git a/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java b/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java
new file mode 100644
index 0000000000..de27df91a3
--- /dev/null
+++ b/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java
@@ -0,0 +1,57 @@
+package bio.terra.service.profile.flight.create;
+
+import bio.terra.model.BillingProfileModel;
+import bio.terra.service.job.JobMapKeys;
+import bio.terra.service.profile.flight.ProfileMapKeys;
+import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentResource;
+import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService;
+import bio.terra.stairway.FlightContext;
+import bio.terra.stairway.Step;
+import bio.terra.stairway.StepResult;
+import java.util.List;
+import java.util.UUID;
+
+public class CreateProfileRegisterDeployedApplicationStep implements Step {
+  private final AzureApplicationDeploymentService applicationDeploymentService;
+
+  /**
+   * This step registers an association between the billing profile created by this flight and its
+   * {@link AzureApplicationDeploymentResource}, and makes it available in its working map.
+   */
+  public CreateProfileRegisterDeployedApplicationStep(
+      AzureApplicationDeploymentService applicationDeploymentService) {
+    this.applicationDeploymentService = applicationDeploymentService;
+  }
+
+  @Override
+  public StepResult doStep(FlightContext context) throws InterruptedException {
+    BillingProfileModel billingProfile = getBillingProfile(context);
+    final AzureApplicationDeploymentResource applicationDeployment =
+        applicationDeploymentService.getOrRegisterApplicationDeployment(billingProfile);
+    context
+        .getWorkingMap()
+        .put(ProfileMapKeys.PROFILE_APPLICATION_DEPLOYMENT, applicationDeployment);
+    return StepResult.getStepResultSuccess();
+  }
+
+  @Override
+  public StepResult undoStep(FlightContext context) throws InterruptedException {
+    UUID profileId = getBillingProfile(context).getId();
+    // We unconditionally delete any registered association between this billing profile and its
+    // application deployment when undoing this step:
+    // there is no circumstance where an association would already exist and be exempt from clean up
+    // because an application deployment can be associated with at most one billing profile.
+    List<UUID> appIdList =
+        applicationDeploymentService.markUnusedApplicationDeploymentsForDelete(profileId);
+    applicationDeploymentService.deleteApplicationDeploymentMetadata(appIdList);
+    return StepResult.getStepResultSuccess();
+  }
+
+  /**
+   * @return the {@link BillingProfileModel} obtained from the working map, assumed to already be
+   *     written as the job's response.
+   */
+  private BillingProfileModel getBillingProfile(FlightContext context) {
+    return context.getWorkingMap().get(JobMapKeys.RESPONSE.getKeyName(), BillingProfileModel.class);
+  }
+}
diff --git a/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java b/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java
index a3ceff9ed4..40f60eec2d 100644
--- a/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java
+++ b/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java
@@ -6,6 +6,7 @@
 import bio.terra.service.job.JobMapKeys;
 import bio.terra.service.journal.JournalService;
 import bio.terra.service.profile.ProfileService;
+import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService;
 import bio.terra.stairway.Flight;
 import bio.terra.stairway.FlightMap;
 import org.springframework.context.ApplicationContext;
@@ -17,6 +18,8 @@ public ProfileCreateFlight(FlightMap inputParameters, Object applicationContext)
 
     ApplicationContext appContext = (ApplicationContext) applicationContext;
     ProfileService profileService = appContext.getBean(ProfileService.class);
+    AzureApplicationDeploymentService azureApplicationDeploymentService =
+        appContext.getBean(AzureApplicationDeploymentService.class);
     JournalService journalService = appContext.getBean(JournalService.class);
 
     BillingProfileRequestModel request =
@@ -33,6 +36,7 @@ public ProfileCreateFlight(FlightMap inputParameters, Object applicationContext)
       addStep(new CreateProfileVerifyAccountStep(profileService, request, user));
     }
     if (platform.isAzure()) {
+      addStep(new CreateProfileRegisterDeployedApplicationStep(azureApplicationDeploymentService));
       addStep(new CreateProfileVerifyDeployedApplicationStep(profileService, request, user));
     }
     addStep(new CreateProfileAuthzIamStep(profileService, request, user));
diff --git a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java
index 33e0eab67e..09fbe4669e 100644
--- a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java
+++ b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java
@@ -56,11 +56,13 @@ public AzureApplicationDeploymentService(
   }
 
   /**
-   * Return or register an application deployment into the TDR metadata
+   * Return the Azure application deployment associated with the {@link BillingProfileModel}, first
+   * registering it in Postgres if it doesn't yet exist.
    *
    * @param billingProfile previously authorized billing profile
-   * @return application deployment resource object
-   * @throws InterruptedException if shutting down
+   * @return the {@link AzureApplicationDeploymentResource} associated with the billing profile
+   * @throws MismatchedBillingProfilesException if another billing profile has already registered
+   *     its association with the Azure application deployment.
    */
   public AzureApplicationDeploymentResource getOrRegisterApplicationDeployment(
       BillingProfileModel billingProfile) {
@@ -141,13 +143,11 @@ public Map<String, String> inputParameters(GenericResource applicationDeployment
   }
 
   /**
-   * Register a new azure application deployment. This process is not transactional or done in a
-   * stairway flight, so it is possible we will allocate projects and before they are recorded in
-   * our database, we will fail and they will be orphaned.
+   * Register and return the Azure application deployment associated with the {@link
+   * BillingProfileModel}.
    *
-   * @param billingProfile authorized billing profile that'll pay for the application deployment
-   * @return a populated application deployment resource object
-   * @throws InterruptedException if the flight is interrupted during execution
+   * @param billingProfile previously authorized billing profile
+   * @return the {@link AzureApplicationDeploymentResource} associated with the billing profile
    */
   private AzureApplicationDeploymentResource newApplicationDeployment(
       BillingProfileModel billingProfile) {
diff --git a/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java b/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java
index 77bdd4d6ea..32d9a7cc82 100644
--- a/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java
+++ b/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java
@@ -34,6 +34,7 @@ void testConstructFlightAzure() {
         contains(
             "GetOrCreateProfileIdStep",
             "CreateProfileMetadataStep",
+            "CreateProfileRegisterDeployedApplicationStep",
             "CreateProfileVerifyDeployedApplicationStep",
             "CreateProfileAuthzIamStep",
             "CreateProfileJournalEntryStep"));
diff --git a/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java b/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java
index d23acc99f9..8e67a985e3 100644
--- a/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java
+++ b/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java
@@ -8,6 +8,11 @@
 import static bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService.STORAGE_TYPE_KEY;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import bio.terra.app.configuration.ApplicationConfiguration;
@@ -18,10 +23,12 @@
 import bio.terra.model.BillingProfileModel;
 import bio.terra.service.resourcemanagement.MetadataDataAccessUtils;
 import bio.terra.service.resourcemanagement.exception.AzureResourceNotFoundException;
+import bio.terra.service.resourcemanagement.exception.MismatchedBillingProfilesException;
 import com.azure.resourcemanager.AzureResourceManager;
 import com.azure.resourcemanager.resources.models.GenericResource;
 import com.azure.resourcemanager.resources.models.GenericResources;
 import java.util.Map;
+import java.util.UUID;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
@@ -42,14 +49,14 @@ class AzureApplicationDeploymentServiceTest {
   private AzureApplicationDeploymentService service;
 
   @BeforeEach
-  void setUp() throws Exception {
+  void setUp() {
     service =
         new AzureApplicationDeploymentService(
             resourceDao, resourceConfiguration, new ApplicationConfiguration().objectMapper());
   }
 
   @Test
-  void testGetOrRegisterApplicationDeployment() {
+  void getOrRegisterApplicationDeployment_register() {
     BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile();
     when(genericResource.properties())
         .thenReturn(
@@ -70,24 +77,72 @@ void testGetOrRegisterApplicationDeployment() {
         .thenThrow(AzureResourceNotFoundException.class);
     when(resourceConfiguration.getClient(billingProfileModel.getSubscriptionId()))
         .thenReturn(client);
+    UUID registrationId = UUID.randomUUID();
+    when(resourceDao.createApplicationDeployment(any())).thenReturn(registrationId);
 
     AzureApplicationDeploymentResource appResource =
         service.getOrRegisterApplicationDeployment(billingProfileModel);
 
+    assertAll(
+        "Verify application deployment properties",
+        () ->
+            assertThat(
+                "Application name matches",
+                appResource.getAzureApplicationDeploymentName(),
+                equalTo(billingProfileModel.getApplicationDeploymentName())),
+        () ->
+            assertThat(
+                "Managed resource group matches",
+                appResource.getAzureResourceGroupName(),
+                equalTo("mgd-grp-1")),
+        () ->
+            assertThat(
+                "Default region matches",
+                appResource.getDefaultRegion(),
+                equalTo(AzureRegion.AUSTRALIA)),
+        () -> assertThat("Prefix matches", appResource.getStorageAccountPrefix(), equalTo("tdr")),
+        () ->
+            assertThat(
+                "Storage account type matches",
+                appResource.getStorageAccountSkuType(),
+                equalTo(AzureStorageAccountSkuType.STANDARD_LRS)),
+        () -> assertThat("Registration ID matches", appResource.getId(), equalTo(registrationId)));
+  }
+
+  @Test
+  void getOrRegisterApplicationDeployment_get() {
+    BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile();
+    AzureApplicationDeploymentResource existingAppResource =
+        new AzureApplicationDeploymentResource()
+            .profileId(billingProfileModel.getId())
+            .azureApplicationDeploymentName("existing-app-resource");
+    when(resourceDao.retrieveApplicationDeploymentByName(
+            billingProfileModel.getApplicationDeploymentName()))
+        .thenReturn(existingAppResource);
+
     assertThat(
-        "Application name matches",
-        appResource.getAzureApplicationDeploymentName(),
-        equalTo(billingProfileModel.getApplicationDeploymentName()));
-    assertThat(
-        "Managed resource group matches",
-        appResource.getAzureResourceGroupName(),
-        equalTo("mgd-grp-1"));
-    assertThat(
-        "Default region matches", appResource.getDefaultRegion(), equalTo(AzureRegion.AUSTRALIA));
-    assertThat("Prefix matches", appResource.getStorageAccountPrefix(), equalTo("tdr"));
-    assertThat(
-        "Storage account type matches",
-        appResource.getStorageAccountSkuType(),
-        equalTo(AzureStorageAccountSkuType.STANDARD_LRS));
+        "Existing application resource for profile is returned",
+        service.getOrRegisterApplicationDeployment(billingProfileModel),
+        equalTo(existingAppResource));
+    verify(resourceDao, never()).createApplicationDeployment(any());
+  }
+
+  @Test
+  void getOrRegisterApplicationDeployment_throwsOnMismatchedBillingProfile() {
+    BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile();
+    UUID differentProfileId = UUID.randomUUID();
+    AzureApplicationDeploymentResource existingAppResource =
+        new AzureApplicationDeploymentResource()
+            .profileId(differentProfileId)
+            .azureApplicationDeploymentName("existing-app-resource");
+    when(resourceDao.retrieveApplicationDeploymentByName(
+            billingProfileModel.getApplicationDeploymentName()))
+        .thenReturn(existingAppResource);
+
+    assertThrows(
+        MismatchedBillingProfilesException.class,
+        () -> service.getOrRegisterApplicationDeployment(billingProfileModel),
+        "Throws when an application deployment already exists for a different profile");
+    verify(resourceDao, never()).createApplicationDeployment(any());
   }
 }

From 8d659c43c0ea258a3f5a3152efe1f444a755fe81 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Tue, 11 Jun 2024 18:57:36 -0400
Subject: [PATCH 10/11] Debug failing tests: disable AzureAuthService caching
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Test failures and flakiness seems to happen…
- During dataset creation…
- Specifically in CreateDatasetGetOrCreateStorageAccountStep when getting an existing SA (a new condition when billing profiles are shared, previously a new one was created with each test case)…
- Specifically in the call to azureBlobStorePdao.enableFileLogging
---
 .../resourcemanagement/azure/AzureAuthService.java       | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java
index 6441b056f9..f6d42fd91f 100644
--- a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java
+++ b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java
@@ -43,7 +43,10 @@ public AzureAuthService(AzureResourceConfiguration configuration) {
         new RequestRetryOptions(
             RetryPolicyType.EXPONENTIAL, maxRetries, retryTimeoutSeconds, null, null, null);
     // wrap the cache map with a synchronized map to safely share the cache across threads
-    authorizedMap = Collections.synchronizedMap(new PassiveExpiringMap<>(15, TimeUnit.MINUTES));
+    // DO NOT MERGE: disabling cache to debug AzureIntegrationTest failures with shared billing
+    // profile.
+    authorizedMap =
+        Collections.synchronizedMap(new PassiveExpiringMap<>(0 /*15*/, TimeUnit.MINUTES));
   }
 
   /**
@@ -152,12 +155,14 @@ public BlobServiceClient getBlobServiceClient(
   /** Obtain a secret key for the associated storage account */
   private String getStorageAccountKey(
       UUID subscriptionId, String resourceGroupName, String storageAccountResourceName) {
-
     AzureAuthorizedCacheKey authorizedCacheKey =
         new AzureAuthorizedCacheKey(subscriptionId, resourceGroupName, storageAccountResourceName);
+    logger.info("Will get or compute AzureAuthService.getStorageAccountKey...");
     return authorizedMap.computeIfAbsent(
         authorizedCacheKey,
         val -> {
+          // DO NOT MERGE: debugging logging only.
+          logger.info("COMPUTING A NEW AzureAuthService.getStorageAccountKey");
           AzureResourceManager client = configuration.getClient(subscriptionId);
           return client
               .storageAccounts()

From ad4aeced5d37ddb4f5898141dc6d4ffb09086158 Mon Sep 17 00:00:00 2001
From: Olivia Kotsopoulos <okotsopo@broadinstitute.org>
Date: Wed, 12 Jun 2024 09:08:54 -0400
Subject: [PATCH 11/11] Explicitly null out resource identifiers on set-up

Otherwise, test tear-down for a different test may unsuccessfully try to delete them -- observed for releaseSnapshotId specifically, which is not initialized in all tests.
---
 src/test/java/bio/terra/integration/AzureIntegrationTest.java | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
index 64c513778e..cc3f2be6c3 100644
--- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java
+++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java
@@ -196,9 +196,12 @@ public void testSetup() {
             null,
             retryOptions);
     gcsBlobIOTestUtility = new GcsBlobIOTestUtility(testConfig.getIngestbucket(), null);
+    datasetId = null;
+    releaseSnapshotId = null;
     snapshotIds = new ArrayList<>();
     snapshotAccessRequestIds = new ArrayList<>();
     storageAccounts = new TreeSet<>();
+    dac = null;
   }
 
   @AfterEach