Skip to content

Commit 1f9cf4c

Browse files
wheels-bot[bot]github-actions[bot]bpamiri
authored
fix(test): resolve BrowserTest base URL through layered lookup at instance time (#2783)
* fix(test): resolve BrowserTest base URL through layered lookup at instance time Specs running against a non-default port (Titan on 60050, scaffolds on 60080) previously had to compare getBaseUrl() against a sentinel string and override it manually because $resolveBaseUrl() returned http://localhost:8080 unconditionally and the only escape hatch (WHEELS_BROWSER_TEST_BASE_URL) is cached by the JVM at process start. $resolveBaseUrl() now consults, in order: this.baseUrl per-spec override, get("browserTestBaseUrl") Wheels setting, -Dwheels.browserTest.baseUrl JVM property, WHEELS_BROWSER_TEST_BASE_URL env, $detectBaseUrlFromCgi() auto-detect (the test runner reaches the suite over HTTP, so cgi already names the right host:port), then the localhost:8080 default. The CGI auto-detect skips when port==8080 so existing default-port runs are unchanged. Fixes #2779 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * docs(web/guides): document BrowserTest layered base-URL resolution (#2779) Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(test): address Reviewer A/B consensus findings (round 1) - Strip canonical ports (http:80, https:443) in $detectBaseUrlFromCgi to match URL conventions; update the https:443 spec expectation and add a dedicated http:80 case (vendor/wheels/wheelstest/BrowserTest.cfc:297-299, vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:51-61). - Document why the "falls back through layers" assertion is intentionally weak — JVM env vars are read-only from CFML and the Wheels get() setting needs a live framework context, so layer isolation isn't fully testable at that level (vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:28-43). - Update browser-test guides (v4-0-0 + v4-0-1-snapshot L319) to recommend setting this.baseUrl in the component pseudo-constructor instead of beforeAll — super.beforeAll() calls $resolveBaseUrl() before a beforeAll-override can take effect, silently inheriting layer 2-6 results. - Mirror the same ordering note in .ai/wheels/testing/browser-testing.md. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --------- Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Peter Amiri <peter@alurium.com>
1 parent a3fb879 commit 1f9cf4c

6 files changed

Lines changed: 186 additions & 15 deletions

File tree

.ai/wheels/testing/browser-testing.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,6 @@ bash tools/test-local.sh # skips browser specs if JARs missin
6464
- **`client` is a Lucee reserved scope.** `var client = ...` in a closure throws "client scope is not enabled". Use `var c = ...` or `var bc = ...`. (Generalized rule: see CLAUDE.md anti-pattern #11.)
6565
- **Data URLs work for most tests** — no server needed for ~95% of DSL coverage. Full HTTP integration (cookies, form submits, redirects) needs a running fixture app; that wiring is the same as Wheels Web app bootstrap (separate server + baseUrl).
6666
- **`this.browserTestSkipped`** — when Playwright JARs aren't installed (fresh CI, clean machine), `beforeAll` sets this flag and `browserDescribe`'s hooks short-circuit. All `it`s should check `if (this.browserTestSkipped) return;` to stay green on CI.
67-
- **CI runs browser tests**`pr.yml` and `snapshot.yml` install Playwright JARs + Chromium (cached via `browser-manifest.json` hash). Browser specs run as part of the normal test suite. `WHEELS_BROWSER_TEST_BASE_URL=http://localhost:60007` is set automatically.
67+
- **CI runs browser tests**`pr.yml` and `snapshot.yml` install Playwright JARs + Chromium (cached via `browser-manifest.json` hash). Browser specs run as part of the normal test suite. `WHEELS_BROWSER_TEST_BASE_URL=http://localhost:60007` is set automatically. The base URL is resolved at instance time through a layered lookup (`this.baseUrl` → Wheels setting → JVM property `wheels.browserTest.baseUrl` → env var → CGI auto-detect → `http://localhost:8080`); per-spec `this.baseUrl` takes priority over the env var. Set `this.baseUrl` in the component pseudo-constructor (outside any function), not inside `beforeAll()``super.beforeAll()` calls `$resolveBaseUrl()` and caches the result, so a `this.baseUrl =` assignment that runs after `super.beforeAll()` is silently ignored.
6868
- **Fixture routes**`/_browser/login-as` and `/_browser/logout` are mounted automatically in test mode. They must come before `.wildcard()` in routes.cfm. In the Routes UI (`/wheels/routes`) all `/_browser/*` routes appear under the **Internal** tab, not Application.
6969
- **Dialogs are Lucee-only**`acceptDialog`, `dismissDialog`, `dialogMessage` use `createDynamicProxy` which is Lucee-specific. Specs skip gracefully on other engines.

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo
2323
### Fixed
2424

2525
- `wheels.wheelstest.BrowserTest` now throws a clear `Wheels.BrowserTest.NotWired` error — naming `browserDescribe()` as the fix — when a spec calls a DSL method on `this.browser` from a plain `describe()` block. Previously the uninitialized `this.browser` was an empty string, producing the misleading `function [visitUrl] does not exist in the String` on every newcomer's first BrowserTest spec. A sentinel `UnwiredBrowserGuard` is now installed at `this.browser` before `browserDescribe()` wires the real `BrowserClient` and after `$endBrowserContext()` tears it down
26+
- `BrowserTest`'s default base URL is no longer hardcoded to `http://localhost:8080`. `$resolveBaseUrl()` now consults a layered lookup at instance time: `this.baseUrl` (per-spec override, set in the component pseudo-constructor) → `get("browserTestBaseUrl")` (Wheels setting) → `-Dwheels.browserTest.baseUrl=...` (JVM system property) → `WHEELS_BROWSER_TEST_BASE_URL` env var → `cgi.server_name`/`cgi.server_port` auto-detect → `http://localhost:8080` default. Specs running against a non-8080 server (Titan on 60050, `wheels new` scaffolds on 60080) can set `this.baseUrl` in the pseudo-constructor or rely on the CGI auto-detect instead of comparing `getBaseUrl()` against a sentinel string. The bare-env-var approach still works for CI but is no longer the only escape hatch (the JVM caches env vars at process start, so post-launch `export` had no effect). Regression spec at `vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc` (#2779)
2627
- Linux `.deb` / `.rpm` packages double-nested the framework at `/opt/wheels/module/vendor/wheels/wheels/` instead of `/opt/wheels/module/vendor/wheels/`. `wheels-core-VER.zip` carries a top-level `wheels/` directory that `unzip` preserves; the nfpm `type: tree` rule then copied the entire `build/framework/` tree (wrapper and all) into the destination, leaving `Injector.cfc` one level too deep. Every fresh `wheels new` install on Ubuntu/Fedora then crashed on first request with `could not find component or class with name [wheels.Injector]`, cascading into the cryptic `The key [WO] does not exist.` error in `onError`. The brew formula handles this correctly via `(share/"wheels/framework/wheels").install Dir["*"]`; the Linux nfpm configs now pin `src` at `./build/framework/wheels/` to match. Regression spec at `vendor/wheels/tests/specs/cli/LinuxPackageStagingSpec.cfc` (#2773)
2728
- `onError` in the generated app template and demo `public/Application.cfc` now guards `application.wo` with `StructKeyExists(application, "wo")` after the recovery try/catch. When `new wheels.Injector(...)` fails during `onApplicationStart` (e.g. a stale `/wheels` mapping under Lucee Express 7), the original error is preserved via a minimal HTML fallback instead of cascading into the cryptic "The key [WO] does not exist" exception that hit "Your First 15 Minutes" tutorial users on fresh installs
2829

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/**
2+
* Regression for #2779: BrowserTest's default base URL is hardcoded to
3+
* http://localhost:8080 and the only escape hatch (the JVM-cached
4+
* WHEELS_BROWSER_TEST_BASE_URL env var) cannot be changed after server
5+
* start. Specs running against non-8080 servers must apply a manual
6+
* fallback comparing getBaseUrl() to a sentinel string.
7+
*
8+
* After the fix, BrowserTest resolves the base URL through a layered
9+
* lookup at instance time:
10+
* 1. this.baseUrl (per-spec override)
11+
* 2. get("browserTestBaseUrl") (Wheels setting)
12+
* 3. JVM system property (wheels.browserTest.baseUrl)
13+
* 4. Env var (WHEELS_BROWSER_TEST_BASE_URL)
14+
* 5. CGI auto-detect (cgi.server_name + cgi.server_port)
15+
* 6. Default (http://localhost:8080)
16+
*/
17+
component extends="wheels.WheelsTest" {
18+
19+
function run() {
20+
describe("BrowserTest base URL layered resolution (issue ##2779)", () => {
21+
22+
it("honors this.baseUrl as the highest-precedence override", () => {
23+
var bt = new wheels.wheelstest.BrowserTest();
24+
bt.baseUrl = "http://override.example:9999";
25+
expect(bt.$resolveBaseUrl()).toBe("http://override.example:9999");
26+
});
27+
28+
it("falls back through layers when this.baseUrl is empty", () => {
29+
// Intentionally weak assertion: JVM env vars are read-only
30+
// from CFML and the Wheels get() setting requires a live
31+
// framework context, so we can't fully isolate which layer
32+
// (2–6) fires here. The JVM-property test below and the
33+
// direct $detectBaseUrlFromCgi tests cover layers 3 and 5
34+
// in isolation. This case only verifies the chain
35+
// terminates with a valid http(s):// URL — the original bug
36+
// was returning the wrong port, not an invalid value.
37+
var bt = new wheels.wheelstest.BrowserTest();
38+
bt.baseUrl = "";
39+
expect(bt.$resolveBaseUrl()).toMatch("^https?://");
40+
});
41+
42+
it("$detectBaseUrlFromCgi returns blank when port is the stale 8080 default", () => {
43+
var bt = new wheels.wheelstest.BrowserTest();
44+
var fakeCgi = {server_port: "8080", server_name: "localhost", https: "off"};
45+
expect(bt.$detectBaseUrlFromCgi(fakeCgi)).toBe("");
46+
});
47+
48+
it("$detectBaseUrlFromCgi builds the URL from a non-default port", () => {
49+
var bt = new wheels.wheelstest.BrowserTest();
50+
var fakeCgi = {server_port: "60050", server_name: "localhost", https: "off"};
51+
expect(bt.$detectBaseUrlFromCgi(fakeCgi)).toBe("http://localhost:60050");
52+
});
53+
54+
it("$detectBaseUrlFromCgi honors the https scheme when cgi.https is on", () => {
55+
var bt = new wheels.wheelstest.BrowserTest();
56+
var fakeCgi = {server_port: "443", server_name: "staging.example.com", https: "on"};
57+
expect(bt.$detectBaseUrlFromCgi(fakeCgi)).toBe("https://staging.example.com");
58+
});
59+
60+
it("$detectBaseUrlFromCgi omits canonical http port 80", () => {
61+
var bt = new wheels.wheelstest.BrowserTest();
62+
var fakeCgi = {server_port: "80", server_name: "example.com", https: "off"};
63+
expect(bt.$detectBaseUrlFromCgi(fakeCgi)).toBe("http://example.com");
64+
});
65+
66+
it("$detectBaseUrlFromCgi returns blank when server_port is missing or zero", () => {
67+
var bt = new wheels.wheelstest.BrowserTest();
68+
expect(bt.$detectBaseUrlFromCgi({server_port: "0"})).toBe("");
69+
expect(bt.$detectBaseUrlFromCgi({})).toBe("");
70+
});
71+
72+
it("JVM system property is honored when this.baseUrl is empty", () => {
73+
var bt = new wheels.wheelstest.BrowserTest();
74+
bt.baseUrl = "";
75+
var sys = createObject("java", "java.lang.System");
76+
var key = "wheels.browserTest.baseUrl";
77+
var prior = sys.getProperty(key);
78+
try {
79+
sys.setProperty(key, "http://jvm-prop.example:1234");
80+
expect(bt.$resolveBaseUrl()).toBe("http://jvm-prop.example:1234");
81+
} finally {
82+
if (isNull(prior)) {
83+
sys.clearProperty(key);
84+
} else {
85+
sys.setProperty(key, prior);
86+
}
87+
}
88+
});
89+
90+
});
91+
}
92+
}

vendor/wheels/wheelstest/BrowserTest.cfc

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ component extends="wheels.WheelsTest" {
4444
// "function [X] does not exist in the String".
4545
this.browser = new wheels.wheelstest.UnwiredBrowserGuard();
4646
this.browserTestSkipped = false;
47+
// Per-spec base-URL override (issue #2779). Highest-precedence layer in
48+
// $resolveBaseUrl(). Set in the component pseudo-constructor (the
49+
// `this.baseUrl = "..."` line outside any function in the subclass),
50+
// NOT inside beforeAll() — super.beforeAll() calls $resolveBaseUrl() and
51+
// caches the resolved URL before any beforeAll-level assignment can take
52+
// effect, so a late override is silently ignored.
53+
this.baseUrl = "";
4754

4855
variables.$launcher = "";
4956
variables.$browser = "";
@@ -232,26 +239,73 @@ component extends="wheels.WheelsTest" {
232239
}
233240

234241
/**
235-
* Base URL for `client.visit("/path")`. Defaults to localhost:8080
236-
* (the default LuCLI port); override via WHEELS_BROWSER_TEST_BASE_URL env.
242+
* Base URL for `client.visit("/path")`. Resolved through a layered lookup
243+
* at instance time so post-launch overrides take effect (the JVM caches
244+
* env vars at process start, so `export`-ing after `wheels start` is too
245+
* late for the bare-env-var approach). Precedence, highest first:
246+
*
247+
* 1. this.baseUrl — per-spec override
248+
* 2. get("browserTestBaseUrl") — Wheels setting
249+
* 3. -Dwheels.browserTest.baseUrl=... — JVM system property
250+
* 4. WHEELS_BROWSER_TEST_BASE_URL env — CI / shell
251+
* 5. $detectBaseUrlFromCgi(cgi) — derived from the in-flight
252+
* test-runner request
253+
* 6. "http://localhost:8080" default — bare LuCLI port
254+
*
237255
* For specs that use only `visitUrl(absolute)` (e.g. data:/file:), the
238256
* baseUrl is effectively unused.
239257
*/
240-
private string function $resolveBaseUrl() {
258+
public string function $resolveBaseUrl() {
259+
if (len(this.baseUrl ?: "")) return this.baseUrl;
260+
261+
try {
262+
var setting = get(name="browserTestBaseUrl");
263+
if (len(setting ?: "")) return setting;
264+
} catch (any e) {
265+
// Setting not registered — fall through to next layer.
266+
}
267+
241268
try {
242-
var env = createObject("java", "java.lang.System")
243-
.getenv("WHEELS_BROWSER_TEST_BASE_URL");
269+
var sys = createObject("java", "java.lang.System");
270+
var prop = sys.getProperty("wheels.browserTest.baseUrl");
271+
if (!isNull(prop) && len(prop)) return prop;
272+
var env = sys.getenv("WHEELS_BROWSER_TEST_BASE_URL");
244273
if (!isNull(env) && len(env)) return env;
245274
} catch (any e) {
246-
// Best-effort: SecurityManager could deny env access. Falling
247-
// back to the localhost default is correct — if the user actually
248-
// configured a different URL but we can't read it, their tests
249-
// will fail with connection-refused, surfacing the problem
250-
// clearly rather than silently using the wrong URL.
275+
// Best-effort: a SecurityManager could deny system access.
276+
// Falling through to CGI detection / default is correct.
251277
}
278+
279+
try {
280+
var detected = $detectBaseUrlFromCgi(cgi);
281+
if (len(detected)) return detected;
282+
} catch (any e) {
283+
// cgi scope unavailable (rare; e.g. background thread) — fall
284+
// through to the hardcoded default.
285+
}
286+
252287
return "http://localhost:8080";
253288
}
254289

290+
/**
291+
* Derive base URL from the in-flight test-runner request. The core test
292+
* suite runs over HTTP at /wheels/core/tests, so the cgi scope already
293+
* names the correct host:port for the running server. Returns "" when
294+
* cgi looks like the bare LuCLI default (localhost:8080) so downstream
295+
* layers can supply the real value.
296+
*/
297+
public string function $detectBaseUrlFromCgi(required any cgiScope) {
298+
if (!structKeyExists(arguments.cgiScope, "server_port") || !val(arguments.cgiScope.server_port ?: 0)) {
299+
return "";
300+
}
301+
var port = val(arguments.cgiScope.server_port);
302+
var host = len(arguments.cgiScope.server_name ?: "") ? arguments.cgiScope.server_name : "localhost";
303+
var scheme = (arguments.cgiScope.https ?: "off") == "on" ? "https" : "http";
304+
if (host == "localhost" && port == 8080) return "";
305+
var isCanonicalPort = (scheme == "http" && port == 80) || (scheme == "https" && port == 443);
306+
return scheme & "://" & host & (isCanonicalPort ? "" : ":" & port);
307+
}
308+
255309
/**
256310
* Best-effort capture of screenshot + HTML on test failure.
257311
* Called from aroundEach catch block. Swallows all errors to avoid

web/sites/guides/src/content/docs/v4-0-0/testing/browser-tests.mdx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ component extends="wheels.wheelstest.BrowserTest" {
9696

9797
`this.browser` is a `BrowserClient`. Every method that mutates state returns `this`, so calls chain fluently until you hit a terminal (a method that returns a value — `currentUrl()`, `text()`, etc.).
9898

99-
- `visit(path)` — navigate to a path joined to the base URL (`http://localhost:8080` by default, or `WHEELS_BROWSER_TEST_BASE_URL`). Path must start with `/`.
99+
- `visit(path)` — navigate to a path joined to the base URL, resolved at instance time through a layered lookup: `this.baseUrl` (per-spec override) → `get("browserTestBaseUrl")` (Wheels setting) → JVM system property `wheels.browserTest.baseUrl``WHEELS_BROWSER_TEST_BASE_URL` env var → CGI auto-detect → `http://localhost:8080` default. Path must start with `/`.
100100
- `visitUrl(url)` — navigate to an absolute URL. Use for `data:`, `file:`, or cross-origin tests.
101101
- `visitRoute(route, key, params)` — resolve a named route via `URLFor()` and navigate there. Requires the Wheels app to be booted.
102102
- `back()` / `forward()` / `refresh()` — drive the browser history.
@@ -316,7 +316,19 @@ When a spec fails, the `aroundEach` hook captures a screenshot and HTML dump to
316316

317317
The CI workflow sets `WHEELS_CI=true`, which `BrowserTest.beforeAll` interprets as an opt-out from running browser specs (they're skipped via `this.browserTestSkipped = true`). To run browser specs in CI, also set `WHEELS_BROWSER_CI_ENABLE=true` and ensure Playwright is installed on the runner. CI caches the Playwright JARs and Chromium via the `browser-manifest.json` hash; first runs pay the download cost once.
318318

319-
Set `WHEELS_BROWSER_TEST_BASE_URL` so `visit("/path")` resolves to the right origin on the CI runner. For local work, the default `http://localhost:8080` matches `wheels server run --port=8080`. See [CI Integration](/v4-0-0/testing/ci-integration/) for the full workflow.
319+
Set `WHEELS_BROWSER_TEST_BASE_URL`, set `this.baseUrl` in the component pseudo-constructor, or rely on CGI auto-detect so `visit("/path")` resolves to the right origin. The env var is the standard CI escape hatch; `this.baseUrl` takes priority and lets individual specs target a different port without touching the environment. Set it in the pseudo-constructor — outside any function — so it is in place before `super.beforeAll()` runs `$resolveBaseUrl()` and caches the value:
320+
321+
```cfm
322+
component extends="wheels.wheelstest.BrowserTest" {
323+
this.baseUrl = "http://staging:60050"; // pseudo-constructor — set before super.beforeAll()
324+
325+
function run() {
326+
// specs ...
327+
}
328+
}
329+
```
330+
331+
For local work against a non-8080 server (Titan on 60050, `wheels new` scaffolds on 60080), CGI auto-detect picks up the correct host and port automatically. See [CI Integration](/v4-0-0/testing/ci-integration/) for the full workflow.
320332

321333
## When to skip browser tests
322334

0 commit comments

Comments
 (0)