Skip to content

DT-3208: Run Synthetic Integration Tests as Unit Tests - #2891

Merged
rushtong merged 18 commits into
developfrom
gr-integration
May 4, 2026
Merged

DT-3208: Run Synthetic Integration Tests as Unit Tests#2891
rushtong merged 18 commits into
developfrom
gr-integration

Conversation

@rushtong

@rushtong rushtong commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Addresses

https://broadworkbench.atlassian.net/browse/DT-3208

Summary

Previously, integration/smoke tests were:

  • Gated behind a separate Maven profile (-P integration-tests)
  • Run in a dedicated GitHub Actions workflow (smoke-tests.yaml) that spun up a full Broad BEE environment
  • Excluded from the normal test run via a Surefire rule
  • Dependent on external infrastructure (Sam, ECM, etc.) components deployed alongside the app

This approach was slow, expensive, and fragile — a BEE outage meant no smoke coverage. A quick scan of recent coverage workflow runs shows no significant difference in run times between this branch and develop which means we're saving 100% of the time spent in the smoke test action and gaining slightly more coverage in this PR.

Changes

What we lose

It is important to call out the one feature smoke tests did previously which was to upload test results to a big query table. This was originally built to support pact testing but never gained traction. It was cumbersome to configure correctly due to xml formatting requirements and we haven't looked at the data in years. I see no value in maintaining it.

Test infrastructure (ContainerTests)

  • New abstract base class that boots the full Dropwizard application in-process using DropwizardAppExtension against consent-ci.yaml
  • All external services (Sam, ECM, GCS, OIDC) are stubbed by a WireMock server on port 9999 — no live infrastructure required
  • Database is seeded once before any tests via typed DAO calls (UserDAO, InstitutionDAO, UserRoleDAO, DacDAO) with 7 synthetic CI users, an institution, role assignments, and a DAC
  • DAOTestHelper (existing TestExecutionListener) automatically provides a Testcontainers Postgres instance in all environments

Test coverage

  • StatusTests — unauthenticated liveness/readiness/version smoke tests that replace the original smoke tests
  • UserTests — new authenticated GET /api/user/me exercised for every seeded CI user; validates the full auth stack (OAUTH2 claim headers → ClaimsCache → DuosUserAuthenticator → Sam/ECM stubs)

Removed

  • smoke-tests.yaml GitHub Actions workflow (BEE-based)
  • integration-tests and all-tests Maven profiles + the corresponding Surefire rule
  • IntegrationTestHelper (superseded by ContainerTests)

Updated

  • pom.xml — profiles removed; integration tests now run unconditionally with all other tests
  • DEVNOTES.md / integration/README.md — updated docs to reflect the new run model

How to run

# All tests (unit + integration) — same as CI
mvn clean test

# Integration tests only
mvn clean test -Dtest="org.broadinstitute.consent.integration.**"

Have you read CONTRIBUTING.md lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@rushtong rushtong changed the title WIP: Integration test replacement DT-3208: Run Synthetic Integration Tests as Unit Tests May 1, 2026
@rushtong
rushtong marked this pull request as ready for review May 1, 2026 22:49
@rushtong
rushtong requested a review from a team as a code owner May 1, 2026 22:49
@rushtong
rushtong requested review from Copilot, fboulnois, kevinmarete and otchet-broad and removed request for a team May 1, 2026 22:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to move the existing “smoke/integration” coverage into the normal mvn test run by running a full in-process Dropwizard application with stubbed external dependencies, and removing the separate BEE-based smoke-test workflow/profile setup.

Changes:

  • Adds a ContainerTests base class that boots the app via DropwizardAppExtension, runs WireMock on :9999, and seeds synthetic baseline DB data.
  • Reworks/introduces integration tests (StatusTests, UserTests) to hit live HTTP endpoints (/status, /liveness, /version, /api/user/me) against the in-process app.
  • Removes the dedicated smoke-test GitHub Actions workflow and the Maven profiles that previously gated integration tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/test/resources/consent-ci.yaml Adds Dropwizard config intended for in-process integration tests.
src/test/java/org/broadinstitute/consent/integration/ContainerTests.java New base class to boot the app, start WireMock, and seed the DB for integration tests.
src/test/java/org/broadinstitute/consent/integration/status/StatusTests.java Switches status endpoint smoke checks to run against the in-process app.
src/test/java/org/broadinstitute/consent/integration/user/UserTests.java Adds authenticated /api/user/me coverage across seeded synthetic users with Sam/ECM stubs.
src/test/java/org/broadinstitute/consent/integration/README.md Updates integration-test documentation to describe the new execution model.
src/test/java/org/broadinstitute/consent/integration/IntegrationTestHelper.java Removes the old helper that hit a deployed environment over HTTP.
pom.xml Removes the Maven profiles that previously configured/isolated integration tests.
DEVNOTES.md Documents how integration tests now run and how the DB is provided.
.github/workflows/smoke-tests.yaml Deletes the BEE-based smoke-test workflow.
Comments suppressed due to low confidence (1)

pom.xml:45

  • The PR removes the only Maven Surefire configuration (previously in profiles). This drops the Mockito -javaagent argLine that enables static/constructor mocking (e.g., tests using mockStatic / mockConstruction), which will likely break the existing unit test suite. Please add an explicit maven-surefire-plugin configuration under <build><plugins> that preserves the prior argLine behavior (including JaCoCo’s argLine) so Mockito instrumentation remains enabled.
  <properties>
    <java.version>25</java.version>
    <maven.compiler.source>25</maven.compiler.source>
    <maven.compiler.target>25</maven.compiler.target>
    <maven.version>3.9.11</maven.version>
    <liquibase.version>4.33.0</liquibase.version>
    <dropwizard.version>5.0.1</dropwizard.version>
    <logback.version>1.5.25</logback.version>
    <postgres.version>42.7.10</postgres.version>
    <surefire.version>3.5.5</surefire.version>
    <swagger.ui.version>5.32.5</swagger.ui.version>
    <swagger.ui.path>META-INF/resources/webjars/swagger-ui/${swagger.ui.version}/</swagger.ui.path>
    <wiremock.version>3.13.2</wiremock.version>
    <junit.version>6.0.3</junit.version>
    <mockito.version>5.23.0</mockito.version>
    <sonar.plugin.version>5.6.0.6792</sonar.plugin.version>
    <netty.override.version>4.2.12.Final</netty.override.version>
    <spotless.version>3.4.0</spotless.version>
    <jetty.version>12.1.8</jetty.version>

    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
    <sonar.organization>broad-databiosphere</sonar.organization>
    <sonar.projectKey>DataBiosphere_consent</sonar.projectKey>
    <sonar.projectName>consent</sonar.projectName>
  </properties>

  <build>
    <testResources>
      <testResource>
        <directory>src/test/resources</directory>
        <filtering>true</filtering>
      </testResource>
      <testResource>

Comment thread src/test/java/org/broadinstitute/consent/integration/ContainerTests.java Outdated
Comment thread src/test/java/org/broadinstitute/consent/integration/README.md Outdated
Comment thread src/test/java/org/broadinstitute/consent/integration/README.md Outdated
Comment thread src/test/java/org/broadinstitute/consent/integration/status/StatusTests.java Outdated
Comment thread src/test/java/org/broadinstitute/consent/integration/user/UserTests.java Outdated
Comment thread DEVNOTES.md Outdated
@sonarqubecloud

sonarqubecloud Bot commented May 1, 2026

Copy link
Copy Markdown

@fboulnois fboulnois left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

database:
driverClass: org.postgresql.Driver
user: test
password: test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the configuration of TestContainer's postgres implementation. No secret here.

@rushtong
rushtong merged commit 15dd564 into develop May 4, 2026
11 checks passed
@rushtong
rushtong deleted the gr-integration branch May 4, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants