Skip to content

fix(ocpp-wamp): fix the client races behind the flaky ocpp-wamp tests - #147

Open
juherr wants to merge 4 commits into
IZIVIA:devfrom
juherr:juherr/ocpp-toolkit-108
Open

juherr wants to merge 4 commits into
IZIVIA:devfrom
juherr:juherr/ocpp-toolkit-108

Conversation

@juherr

@juherr juherr commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Closes #108

AutoReconnectOcppWampClient now waits for the reconnection, within callTimeout, when a call races a dropped connection. The underlying client signals that case with a new NotConnectedException, raised before anything is sent. OkHttpOcppWampClient reads its connection when a message is received, so a call result that arrives right before the server closes is no longer dropped. This lost result made both shutdown tests from #122 fail: the in-flight one fails 6 out of 6 isolated runs on dev and 0 out of 6 here. WampLoadTest moves out of check into ./gradlew :ocpp-wamp:loadTest, and each fix has a test that forces its window open (1214 tests green, 10 runs of the module under CPU load green).

The OkHttp client clears its connection before notifying it as lost.
A call made by the auto reconnect client in between still saw it as
connected, skipped the reconnection wait, and failed with
IllegalStateException instead of waiting for the reconnection.

The underlying client now throws NotConnectedException, an
IllegalStateException raised before anything is sent. The auto
reconnect client catches it, waits within the call timeout for the
connection lost to be notified, then retries through the reconnection
path. Reordering the notification would only narrow the window: a call
can still read the connected state just before the drop.

Refs IZIVIA#108
WampLoadTest opens 1000 clients and sends 10000 heartbeats: its outcome
depends on the throughput of the machine, which a shared CI runner does
not guarantee, so it failed builds unrelated to ocpp-wamp.

The default test task now excludes it, and a dedicated loadTest task,
not wired into check, runs it. The exclusion is a class filter rather
than a JUnit tag, since coreProject() configures the JUnit Platform in
a doFirst that could override tag options.

Refs IZIVIA#108
The client handles incoming messages on the default dispatcher, and
read its connection only once the handling ran. When the server closed
the connection right after answering, as shutdown does once in-flight
calls are answered, the close could clear the connection first: the
call result was dropped and the pending call timed out.

This made "shutdown should answer an in-flight inbound call before
closing the connection" fail on every isolated run on dev, where the
first parse of a fresh JVM is slow, and matches how "shutdown should
reject an inbound call received while draining" failed on CI: the
pending call timed out although the shutdown had answered it.

The connection is now read on the websocket thread when the message is
received. The new test holds the dispatcher threads until the close to
open the window.

Refs IZIVIA#108
Condition.await reports a timeout through its return value, which
SonarCloud flags when ignored. The wait now fails the call as soon as
it times out, instead of on the next deadline check: same outcome.

Refs IZIVIA#108
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

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.

ci: :ocpp-wamp:test is flaky and produces unrelated red builds

1 participant