Commits
Tarun Sukhani authored 49cca4f4945
fix: autotest + TestRunnerPlugin always use HTTP for FirePhoque/banner URLs (PF-73, release 1.12.23)
Pre-PF-73 the URL construction in two places branched on `https.port` being
truthy/non-null. PF-72's `%test.https.port=-1` is a non-empty string and
non-null Java reference, so both branches selected it as the port — producing
the unreachable URLs `https://localhost:-1/@tests` and `https://localhost:-1/@kill`.
play autotest then printed "Tests did not successfully complete" because
FirePhoque couldn't connect.
Design intent (per the ticket): tests don't need TLS — the /@tests endpoint
serves identically over plain HTTP, FirePhoque doesn't care about protocol,
and forcing HTTPS in the test path introduces JDK-truststore complexity
(mkcert-signed certs aren't trusted by the JDK by default) and port-collision
risk. So the right answer is to ignore https.port entirely from the test
runner's URL construction.
Fixes:
* framework/pym/play/commands/autotest.py
- Extracted `_firephoqueHttpPort(app)` helper: resolves http.port through
the %test prefix as usual, falls back to "9000" (the framework's default
when nothing is set, per Server.java).
- Hardcoded http:// in both URL constructions (FirePhoque application.url
+ the /@kill request after tests complete).
- Dropped the dead `cert_file` existence guard — structurally unreachable
once we never go down the https branch. The PF-68 truststore-arg comment
block also collapses to a one-line PF-73 note.
* modules/testrunner/src/play/modules/testrunner/TestRunnerPlugin.java
- Same bug pattern in onApplicationReady — null-check on https.port hit
the "-1" sentinel. Hardcoded http and consult only http.port (with 9000
fallback). Banner now reads "Go to http://localhost:9000/@tests" instead
of the broken "https://localhost:-1/@tests".
Tests:
* framework/test-src/cli/test_cli.py — new TestAutotestUrl class with 5 unit
tests on the helper. Covers: https.port=9443, https.port=-1 (the PF-72
default that triggered the bug), https.port=19443, custom http.port=8080,
no http.port (fallback to 9000).
End-to-end verified on /tmp/pf73test for all three AC scenarios in the ticket:
- AC1: https.port=9443 active, no %test override → autotest uses http:9000 ✓
- AC2: %test.https.port=-1 (PF-72 default) → autotest uses http:9000 ✓
- AC3: custom %test.https.port=19443 → autotest uses http:9000 ✓
Both Http2FunctionalTest (3/3) and Http3FunctionalTest (1/1) still green —
the integration testapp's pinned %test.https.port=19443 is unaffected
because the change is to the autotest/TestRunnerPlugin URL construction
only, not to the Server.java port-binding path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>