test: guard per-simulation topic isolation with two concurrent simulations

[craigpnnl]

Adds a regression guard for per-simulation topic isolation, the behaviour
delivered by @poorva1209 in faa2d4e0 via #1758 (merged 2024-01-29). That fix
has been running in develop since January 2024 with no test covering it.

Closes the coverage gap noted when #1757 was closed.

What it guards

SimulationManager and helics_goss_bridge originally shared one topic across
simulations, so concurrent instances corrupted each other's status. The fix
suffixes the simulation id:

• gov.pnnl.goss.gridappsd/src/gov/pnnl/goss/gridappsd/simulation/SimulationManagerImpl.java:224
• gov.pnnl.goss.gridappsd/src/gov/pnnl/goss/gridappsd/simulation/SimulationProcess.java:217,232,255,290
• services/helicsgossbridge/service/helics_goss_bridge.py:440,642,681

Nothing asserted that, so dropping the suffix would ship silently.

The test

SimulationContainerTest.testRunTwoConcurrentSimulationsWithTopicIsolation

Starts two simulations concurrently on separate Client connections (a shared
JMS Session is not safe across threads), subscribes each to its own
topic_simulationOutput.<simId> and topic_simulationLog<simId> using exact
strings rather than wildcards, and asserts every message arriving on simulation
A's subscription carries A's own id and never B's.

How it fails if the suffix is removed: the exact-string subscriptions stop
matching anything the platform publishes, so the "at least one log message
received" assertion fails outright. If the suffix is present but a shared or
wrong id is used, a message intended for the other simulation lands on this
subscription and the per-message id equality assertion fails, printing the
mismatched id.

That failure mode is not hypothetical. The first run failed against a real bug in
the test itself: subscribe() callbacks arrive wrapped in a DataResponse
envelope rather than as raw text. The assertions caught it, which is direct
evidence they are live rather than vacuous.

Concurrency uses a two-thread executor, a CountDownLatch per watcher, and
bounded shared-deadline waits. No sleeps, no polling. Both simulations use the
ieee123 feeder, confirmed present by SPARQL before the run.
TWO_SIM_TIMEOUT_SECONDS = SIMULATION_DURATION + 240.

What it deliberately does NOT assert

Simulation completion. The existing single-simulation test has its completion and
measurement assertions commented out as infrastructure-dependent, and that turns
out to be accurate rather than flaky: running it unmodified gives
Completed: false, Total measurements received: 0, because the simulation dies
about five seconds in at
GLDZiploadScheduleConfigurationHandler.java:209 with
No load schedule data in time series data store. Proven/InfluxDB has no seeded
load-schedule timeseries data, so simulationManager.startSimulation() is never
reached.

That is a real, reproducible environment gap and is tracked separately. Asserting
completion here would produce a test that fails for reasons unrelated to topic
isolation and gets disabled again. The isolation assertion stands on its own and
is the behaviour this PR exists to protect.

Running it without building anything

CI published an image for this branch. Drop this file into
gridappsd-docker/docker-compose.d/ and run ./run.sh -t develop:

services:
  gridappsd:
    image: gridappsd/gridappsd:test_gadp-061-two-simulations
    pull_policy: always

Only the gridappsd service is repointed; blazegraph, viz, proven and influxdb
stay on develop. pull_policy: always matters, since a stale local layer under
the same tag must not win over the registry.

Note that only one docker-compose.d/*.yml override should target the
gridappsd service at a time; disable any other before bringing the stack up.

Image: gridappsd/gridappsd:test_gadp-061-two-simulations, also tagged
26080203_af05161, digest
sha256:4bbf0005b8295caabd0260501e67f444b5f57776e1daf5d17c633360d6b23ab6.
Verified by running the full stack from this override alone, with no bind mounts,
and confirming the container's goss-core jars are 16.0.1.

Results

New test passes; simulation ids distinct; log-topic isolation held. Full
SimulationContainerTest class, 6 tests, passes.

Test code only. No production changes.

An unrelated pre-existing failure surfaces on a full unfiltered containerTest
run: GridAppsDContainerTest.initializationError conflicts on fixed ports when a
stack is already running, because it manages its own Testcontainers lifecycle
while GridAppsDTestEnvironment expects an externally started stack. Two
incompatible strategies share the container tag. Untouched here, reproducible
without this diff.

[github-actions]

Code formatting check failed!

Please run ./gradlew spotlessApply locally to fix formatting issues, then commit the changes.

[Copilot]

Pull request overview

Adds a regression guard to ensure per-simulation topic suffixing remains intact when two simulations are started concurrently, preventing cross-talk on simulation output/log topics. This closes a long-standing coverage gap around the multi-simulation topic isolation fix.

Changes:

• Add SimulationContainerTest.testRunTwoConcurrentSimulationsWithTopicIsolation to assert per-simulation log/output topic isolation under concurrency.
• Add a dedicated make test-simulation-container target to run only the externally-started-stack simulation container tests.
• Wire the new simulation-container test target into the Integration Tests GitHub Actions workflow.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
Makefile	Adds test-simulation-container target and help text to run only simulation-tagged container tests against an externally started stack.
gov.pnnl.goss.gridappsd/test/gov/pnnl/goss/gridappsd/SimulationContainerTest.java	Introduces a two-concurrent-simulation topic-isolation regression test with dedicated topic watchers and DataResponse unwrapping.
.github/workflows/integration-tests.yml	Runs make test-simulation-container in CI after bringing the docker-compose stack up and verifying readiness.

Suppressed comments (1)

gov.pnnl.goss.gridappsd/test/gov/pnnl/goss/gridappsd/SimulationContainerTest.java:479

• This assignment references watcher.expectedSimulationId, but that field is otherwise unused. If the field is removed, this line should be dropped as well.

        watcher.expectedSimulationId = simulationId;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.