Improve runtime, reduce log output, and stabilize SSL failure assertions for `SSLCertificateTest` by Copilot · Pull Request #1215 · quickfix-j/quickfixj

Copilot · 2026-05-06T10:49:36Z

SSLCertificateTest was consuming ~20 of the CI job's ~45 minutes, logging ~45k of ~120k log lines. This PR now includes six targeted fixes:

Timeout and heartbeat tuning

TIMEOUT_SECONDS: 15s → 7s
HeartBtInt: 30s → 5s across all session settings builders — a heartbeat exchange within the window is sufficient to confirm a healthy session

Reconnect interval cleanup

Removed all explicit Initiator.SETTING_RECONNECT_INTERVAL overrides in this test class so the default 30 second reconnect interval is used
This avoids repeated expected SSL exceptions during the 7 second assertion window

Reduce log noise for expected failures

exceptionExpected is now configured at connector construction time and stored as an immutable (final) field on TestConnector
Removed the mutable expectException() path; shouldFailWhen* tests now construct relevant connectors with exceptionExpected=true before start()
Removed late flag mutation from assertSslExceptionThrown(...)
The MINA filter in prepareConnector logs message-only when exceptions are expected, and full stack trace otherwise
Exception logs include connector context (initiator/acceptor and connectPort/acceptPort)
Logging format was normalized with a static prefix first (exceptionCaught ...), and expected-path logs are explicitly marked (exceptionCaught (expected) ...)

This cuts the bulk of log output generated by shouldFailWhen* tests while making the remaining output easier to attribute.

Parallel no-exception checks

Added assertNoSslExceptionsThrown(TestConnector... connectors) — runs each connector's latch-wait concurrently instead of sequentially. For a test with N connectors this replaces N×7s with a single 7s wait.

// before: sequential, each blocks for up to 7s
initiator.assertNoSslExceptionThrown();
// ... initiator assertions ...
acceptor.assertNoSslExceptionThrown();

// after: single concurrent wait
assertNoSslExceptionsThrown(initiator, acceptor);
// ... initiator and acceptor assertions ...

Threads are named (ssl-check<Type><index>) for debuggability. On failure, a composite error message summarises all failures upfront with individual errors as suppressed exceptions. Applied across 15 test methods.

Test name logging

Added a class-level LOGGER and a @Rule TestName field to SSLCertificateTest. The existing @Before setUp() now logs >>> Running test: <methodName> before every test, making it easy to correlate log output with the active test method in CI logs.

Targeted flakiness stabilization for `shouldFailWhenUsingEmptyServerKeyStore()`

Kept strict SSL-exception assertion on the acceptor.
Updated initiator assertion to accept failure as an outcome set:
- SSL exception observed, or
- no SSL exception observed but the initiator did not send FIX Logon before disconnect/reset.
Implemented minimal Logon tracking on TestInitiator via toAdmin(...) (MsgType.LOGON) to support this check.
Extracted this fallback into a dedicated helper assertion (assertSslExceptionThrownOrLogonNotSent(...)) for clarity.

This stabilizes the test across JSSE/provider/timing differences while preserving intended behavior.

…rallelize no-exception checks Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f464dac4-ab03-4de1-a750-fbcb1d6bd19c Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

…rror messages Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f464dac4-ab03-4de1-a750-fbcb1d6bd19c Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

…Caught handler Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/064bbb3a-a6bf-4600-bf29-db71d02c5845 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/d64730b5-aa97-44bd-9061-f58167740bad Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/84ec41ca-7a14-49eb-ae78-d83bfe444cf6 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Refactor createDefaults method to initialize defaults directly.

…lts() overload Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/51422522-fc50-446b-a611-8461f3651a3e Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/7e6f22d7-9852-4fa4-97a1-a398d3d572a3 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot · 2026-05-09T11:28:39Z

@copilot
shouldFailWhenUsingEmptyServerKeyStore sometimes does not see the expected exception for the initiator. Only the acceptor gets the exception reliably.

...

Implemented for shouldFailWhenUsingEmptyServerKeyStore() in 34a7835. The initiator now accepts outcome-set behavior: it passes if an SSL exception is observed, or (if not) asserts that no Logon was sent before disconnect. Acceptor remains strict on SSL exception. I also updated the PR description to document this targeted change.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

localhost-quickfixj
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260509112429257_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-09T11-24-28_446-jvmRun1 surefire-20260509112429257_1tmp surefire_0-20260509112429257_2tmp (dns block)
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260509112544266_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-09T11-25-44_194-jvmRun1 surefire-20260509112544266_1tmp surefire_0-20260509112544266_2tmp (dns block)
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260509112645065_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-09T11-26-45_005-jvmRun1 surefire-20260509112645065_1tmp surefire_0-20260509112645065_2tmp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f0905c7d-0258-433c-be68-686a6ad569d1 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot · 2026-05-11T10:27:36Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

localhost-quickfixj
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260511102601661_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-11T10-26-00_863-jvmRun1 surefire-20260511102601661_1tmp surefire_0-20260511102601661_2tmp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

chrjohn · 2026-05-14T07:46:37Z

@the-thing could you please take a look at this whenever you have the time. Thank you

the-thing · 2026-05-14T08:15:32Z

Sure. More AI slop. We are doomed...

the-thing

Looks ok-ish. Did it improve running time and or differences across different operating systems?

the-thing · 2026-05-14T08:16:29Z

        private final SessionConnector connector;
        private final CountDownLatch exceptionThrownLatch;
        private final AtomicReference<Throwable> exception;
+        private volatile boolean exceptionExpected = false;


This shouldn't be volatile. This should be final and set during construction time. Test case knows beforehand if the exception being thrown is required or not.

Now when I look at this it looks that the flag is used only for logging. I have a feeling it shouldn't be here at all.

the-thing · 2026-05-14T08:17:42Z

+                    String portLabel = getPortLabel(session);
+                    int port = getPort(session);
+                    if (exceptionExpected) {
+                        LOGGER.info("{} {}={} exceptionCaught: {}", connectionType, portLabel, port, cause.getMessage());


For logging, usually the static message comes first and then additional context arguments.

the-thing · 2026-05-14T08:20:47Z

            SSLUtil.assertNotAuthenticated(connector, sessionID, authOn);
        }

+        public void expectException() {


To be removed once the field is final.

the-thing · 2026-05-14T08:21:07Z

        }

        public void assertSslExceptionThrown(String expectedErrorMessage, Class<?> expectedErrorType) throws Exception {
+            exceptionExpected = true;


Needs to be removed.

the-thing · 2026-05-14T08:25:42Z

-                initiator.assertSslExceptionThrown();
+                // Depending on JSSE/provider timing, initiator may fail before surfacing an SSL exception.
+                try {
+                    initiator.assertSslExceptionThrown();


Ok I see what it's doing. When exception is expected but it is not thrown it switches to validate if logon was sent - seems reasonable.

Probably this should be abstracted into a separate asset method e.g. "assertSslExceptionThrownOrLogonNotSent" etc.

chrjohn · 2026-05-14T13:43:22Z

Sure. More AI slop. We are doomed...

Hi @the-thing , IMHO it's not slop as long as someone asked for it. :) I created an issue here #1214 to improve things.
I did not want to ask you, as the creator of the test, to sacrifice your valuable time on the improvement of a test which does its job very well. Thank you as always for your review comments and contributions. 👍
For me as a maintainer using copilot saves me quite some time. I can kick off a task and come back to it later. I think you can tell by the lifetime those PRs exist that it is still taking some time for me to review and merge and that I do not blindly trust everything it generates.

Looks ok-ish. Did it improve running time and or differences across different operating systems?

Complete CI job time is down from about 45 mins to about 30 mins. I did not yet count how many lines of log file less.

the-thing · 2026-05-14T13:54:39Z

No problem. Take it with a pinch of salt. I use AI myself occasionally, but I think it is being used too much.

Complete CI job time is down from about 45 mins to about 30 mins. I did not yet count how many lines of log file less.

That is really good.

chrjohn · 2026-05-15T12:53:53Z

@copilot please integrate proposed review changes by the-thing

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/74ec0d16-c569-477a-80ff-9996ad4af00e Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot · 2026-05-15T13:43:10Z

@copilot please integrate proposed review changes by the-thing

Integrated the proposed review changes from @the-thing in SSLCertificateTest: constructor-time immutable expected-exception configuration (removed mutable toggles), logging format adjustment, and extracted helper assertion for the initiator fallback. Also aligned the mixed multi-session failure test with constructor-based expected exceptions. Verified with SSLCertificateTest passing locally. Commits: 22eac2e, e4ab1cf.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

localhost-quickfixj
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260515130442013_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-15T13-04-41_183-jvmRun1 surefire-20260515130442013_1tmp surefire_0-20260515130442013_2tmp in/m2.conf (dns block)
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260515131434412_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-15T13-14-34_342-jvmRun1 surefire-20260515131434412_1tmp surefire_0-20260515131434412_2tmp (dns block)
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx2g -Djava.net.preferIPv4Stack=true --add-opens java.base/java.lang=ALL-UNNAMED -jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire/surefirebooter-20260515132646872_3.jar /home/REDACTED/work/quickfixj/quickfixj/quickfixj-core/target/surefire 2026-05-15T13-26-46_792-jvmRun1 surefire-20260515132646872_1tmp surefire_0-20260515132646872_2tmp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Initial plan

b0f3ec8

Copilot AI assigned Copilot and chrjohn May 6, 2026

Copilot started work on behalf of chrjohn May 6, 2026 10:49 View session

Copilot AI linked an issue May 6, 2026 that may be closed by this pull request

Runtime and log output for SSLCertificateTest needs to be improved #1214

Open

Copilot AI and others added 2 commits May 6, 2026 11:15

Improve SSLCertificateTest runtime: lower timeout, reduce logging, pa…

1d032d1

…rallelize no-exception checks Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f464dac4-ab03-4de1-a750-fbcb1d6bd19c Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Improve assertNoSslExceptionsThrown: add thread names and composite e…

7dd468d

…rror messages Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f464dac4-ab03-4de1-a750-fbcb1d6bd19c Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Improve runtime for SSLCertificateTest~~ Improve runtime for SSLCertificateTest May 6, 2026

Copilot AI requested a review from chrjohn May 6, 2026 11:21

Copilot finished work on behalf of chrjohn May 6, 2026 11:21

chrjohn requested changes May 6, 2026

View reviewed changes

Comment thread quickfixj-core/src/test/java/quickfix/mina/ssl/SSLCertificateTest.java Outdated

Copilot started work on behalf of chrjohn May 6, 2026 13:39 View session

Log full stack trace only when exception is not expected in exception…

433a0d5

…Caught handler Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/064bbb3a-a6bf-4600-bf29-db71d02c5845 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot finished work on behalf of chrjohn May 6, 2026 13:51

Copilot AI requested a review from chrjohn May 6, 2026 13:51

Copilot started work on behalf of chrjohn May 6, 2026 15:05 View session

Copilot finished work on behalf of chrjohn May 6, 2026 15:07

chrjohn changed the title ~~Improve runtime for SSLCertificateTest~~ Improve runtime for SSLCertificateTest May 6, 2026

chrjohn changed the title ~~Improve runtime for SSLCertificateTest~~ Improve runtime and log output for SSLCertificateTest May 6, 2026

Copilot started work on behalf of chrjohn May 6, 2026 15:24 View session

Unify create*Settings by extracting createCommonDefaults()

978c49d

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/d64730b5-aa97-44bd-9061-f58167740bad Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot finished work on behalf of chrjohn May 6, 2026 15:29

Copilot started work on behalf of chrjohn May 6, 2026 15:45 View session

Rename createCommonDefaults() to createDefaults() (no-arg overload)

a05bc19

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/84ec41ca-7a14-49eb-ae78-d83bfe444cf6 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot finished work on behalf of chrjohn May 6, 2026 15:48

Inlined method createDefaults()

96cf74c

Refactor createDefaults method to initialize defaults directly.

Copilot started work on behalf of chrjohn May 6, 2026 17:04 View session

Revert "Inlined method createDefaults()" - restore no-arg createDefau…

9ea2ec2

…lts() overload Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/51422522-fc50-446b-a611-8461f3651a3e Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot stopped work on behalf of chrjohn due to an error May 6, 2026 17:05
The session was cancelled by the user.

chrjohn changed the title ~~Improve runtime and log output for SSLCertificateTest~~ Improve runtime and reduce log output for SSLCertificateTest May 6, 2026

Catch only FieldNotFound when inspecting initiator admin message type

34a7835

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/7e6f22d7-9852-4fa4-97a1-a398d3d572a3 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot AI changed the title ~~Improve runtime and reduce log output for SSLCertificateTest~~ Improve runtime, reduce log output, and stabilize SSL failure assertions for SSLCertificateTest May 9, 2026

Copilot finished work on behalf of chrjohn May 9, 2026 11:29

chrjohn added 2 commits May 10, 2026 12:51

Merge branch 'master' into copilot/improve-ssl-certificatetest-runtime

e117ff4

Merge branch 'master' into copilot/improve-ssl-certificatetest-runtime

869fbcb

chrjohn reviewed May 11, 2026

View reviewed changes

Comment thread quickfixj-core/src/test/java/quickfix/mina/ssl/SSLCertificateTest.java Outdated

Copilot started work on behalf of chrjohn May 11, 2026 09:50 View session

Add short comment explaining initiator SSL failure fallback approach

1b29ea8

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/f0905c7d-0258-433c-be68-686a6ad569d1 Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot finished work on behalf of chrjohn May 11, 2026 10:27

Copilot AI requested a review from chrjohn May 11, 2026 10:27

chrjohn modified the milestones: QFJ 3.0.1, QFJ 3.0.2 May 12, 2026

chrjohn added 3 commits May 12, 2026 15:23

Merge branch 'master' into copilot/improve-ssl-certificatetest-runtime

ba2d8fe

Merge branch 'master' into copilot/improve-ssl-certificatetest-runtime

ad4347a

Merge branch 'master' into copilot/improve-ssl-certificatetest-runtime

aee767a

the-thing suggested changes May 14, 2026

View reviewed changes

Copilot started work on behalf of chrjohn May 15, 2026 12:30 View session

Copilot finished work on behalf of chrjohn May 15, 2026 12:31

Copilot started work on behalf of chrjohn May 15, 2026 12:54 View session

Copilot AI and others added 2 commits May 15, 2026 13:23

Integrate the-thing SSL test review updates

22eac2e

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/74ec0d16-c569-477a-80ff-9996ad4af00e Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Finalize the-thing review-driven connector expectations

e4ab1cf

Agent-Logs-Url: https://github.com/quickfix-j/quickfixj/sessions/74ec0d16-c569-477a-80ff-9996ad4af00e Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>

Copilot finished work on behalf of chrjohn May 15, 2026 13:44

Conversation

Copilot AI commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Timeout and heartbeat tuning

Reconnect interval cleanup

Reduce log noise for expected failures

Parallel no-exception checks

Test name logging

Targeted flakiness stabilization for shouldFailWhenUsingEmptyServerKeyStore()

Uh oh!

Uh oh!

Copilot AI commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Copilot AI commented May 11, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

chrjohn commented May 14, 2026

Uh oh!

the-thing commented May 14, 2026

Uh oh!

the-thing left a comment

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

the-thing May 14, 2026

Choose a reason for hiding this comment

Uh oh!

chrjohn commented May 14, 2026

Uh oh!

the-thing commented May 14, 2026

Uh oh!

chrjohn commented May 15, 2026

Uh oh!

Copilot AI commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 6, 2026 •

edited

Loading

Targeted flakiness stabilization for `shouldFailWhenUsingEmptyServerKeyStore()`

Copilot AI commented May 9, 2026 •

edited

Loading

Copilot AI commented May 15, 2026 •

edited

Loading