Skip to content

Commit 343ae6f

Browse files
wheels-bot[bot]github-actions[bot]bpamiri
authored
fix(test): auto-bind include-injected globals into WheelsTest spec scope (#2793)
* fix(test): auto-bind include-injected globals into WheelsTest spec scope `WheelsTest`'s pseudo-constructor used `getMetaData(application.wo).functions` to discover which Wheels globals to copy into a spec's `variables`/`this` scope. That metadata enumerates only methods declared on the CFC body and silently skips symbols merged in via `cfinclude` — which is how `vendor/wheels/Global.cfc` pulls user helpers from `app/global/functions.cfm`. Apps with custom helpers (`can()`, `hasRole()`, etc.) had to manually rebind each one in `beforeAll()`. The loop now iterates `application.wo` as a struct and binds every UDF detected by `isCustomFunction()`, while preserving the existing public-only filter for metadata-declared methods and the don't-clobber guard for scope members the spec (or its base class) already provides. Fixes #2790 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(test): address Reviewer A/B consensus findings (round 1) - Promote include-injected UDFs from `variables` to `this` in `vendor/wheels/Global.cfc` after `include "/app/global/functions.cfm"`, so the auto-bind loop in `vendor/wheels/WheelsTest.cfc` discovers them uniformly across Lucee, Adobe CF, and BoxLang. Lucee's struct-iteration over a CFC instance surfaces both `this` and `variables` scopes, but Adobe CF only reliably exposes `this`-scope members — without the promotion, the original bug (#2790) would silently persist on Adobe CF even with the new iteration path in WheelsTest.cfc. - Fix the misleading header comment in `vendor/wheels/tests/specs/wheelstest/WheelsTestAutoBindIncludesSpec.cfc`. Bracket-notation assignment from outside writes to `this` scope, not `variables` — so the probe simulates the post-promotion shape, not the raw include shape. Comment now spells this out explicitly. - Add a new `it` case that asserts the probe key is enumerated by `for (key in application.wo)`. Guards the iteration mechanism the auto-bind loop depends on, so failures on any engine where struct- iteration is narrower than expected would fail this spec rather than silently pass-but-not-test downstream. Addresses Reviewer A's cross-engine concern (Adobe CF struct-iteration contract) and Reviewer B's joint recommendation option (b): promote include-injected helpers to `this` so the iteration path is uniform. The accompanying spec correction handles A's "spec injects via wrong scope" finding and B's "misleading header comment" note. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(test): seed `local` scope before pseudo-constructor for-iterator The promotion loop added in round 1 of the consensus fixup crashed Lucee 7 with `variable [local] doesn't exist` at Global.cfc:3861 — the test runner couldn't even reach a spec before bailing out. In a CFC pseudo-constructor (component body, not inside a function), the `local` scope is not auto-created. Direct assignment to `local.X = ...` will seed it, but `for (local.X in Y)` tries to read `local` first as the iterator's target parent and fails. WheelsTest.cfc gets away with the same loop shape only because it does `local.metaIndex = {}` earlier in its own pseudo-constructor; Global.cfc had no such seeding line. Add the minimum seeding statement (`local.varKey = "";`) directly above the loop and document the cross-engine reason inline. The loop's filter logic is unchanged. The original review couldn't catch this — the round-1 address-review sandbox lacked a working test runner so the fix went out unverified. Signed-off-by: Peter Amiri <peter@alurium.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 acd86f9 commit 343ae6f

4 files changed

Lines changed: 159 additions & 10 deletions

File tree

CHANGELOG.md

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

2323
### Fixed
2424

25+
- `WheelsTest` auto-bind missed user-defined global helpers added via `include` in `app/global/functions.cfm`. The pseudo-constructor used `getMetaData(application.wo).functions`, which only enumerates methods declared directly on the CFC and skips symbols merged in via `cfinclude`. Specs that called custom helpers (e.g. `can()`, `hasRole()`) had to manually rebind each one in `beforeAll()`. The auto-bind now iterates `application.wo` as a struct and binds every UDF via `isCustomFunction()`, preserving the existing public-only filter for declared methods (#2790)
2526
- Model layer SELECT clause builder now routes column identifiers through the adapter's `$quoteIdentifier`, so reserved-word column names (e.g. `key`, `order`, `group`) survive on every supported dialect instead of breaking `findAll` / `findOne` / dynamic finders with cryptic SQL syntax errors. The WHERE / ORDER BY paths already quoted columns; `$createSQLFieldList` and the empty-pagination column-list extraction in `read.cfc` now match.
2627
- `wheels packages install <name>` is now a transparent alias for `wheels packages add <name>` on every caller path that actually reaches `Module.cfc` (stdio MCP server, scripted in-process clients, spec suite). Previously the dispatch layer's `case "install":` branch printed a yellow warning to stdout and returned `""` without installing anything — so even though `PackagesMainCli.install()` itself had been a true alias for `add()` since #2729, any caller routing through the CLI module's verb dispatch silently no-op'd. The shell-facing `wheels packages install <name>` form is still intercepted by LuCLI's built-in extension installer upstream of module dispatch and remains broken on that path (and is documented as such in the module-owned help text), but MCP tool calls and programmatic callers now behave identically to `add`. Both branches now share a single fall-through body so the validation, error shape, and install behavior cannot drift apart again.
2728
- `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

vendor/wheels/Global.cfc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,4 +3852,27 @@ return local.$wheels;
38523852
// User-defined global functions
38533853
include "/app/global/functions.cfm";
38543854

3855+
// Promote include-injected UDFs from `variables` to `this` so they're
3856+
// discoverable via struct-iteration on engines (Adobe CF) where only
3857+
// `this`-scope members are reliably enumerable. Declared methods on
3858+
// Global.cfc are already in `this` via their `access` modifier and are
3859+
// not clobbered by the `structKeyExists(this, ...)` guard. See #2790
3860+
// and the auto-bind loop in `vendor/wheels/WheelsTest.cfc`.
3861+
//
3862+
// The leading `local.varKey = ""` seeds the `local` scope: Lucee 7's
3863+
// pseudo-constructor doesn't auto-create `local` for the iterator
3864+
// target of `for (local.X in Y)`, throwing "variable [local] doesn't
3865+
// exist" without a prior assignment. The same pattern is used in
3866+
// `WheelsTest.cfc` (which seeds `local.metaIndex = {}` before its loop).
3867+
local.varKey = "";
3868+
for (local.varKey in variables) {
3869+
if (!isCustomFunction(variables[local.varKey])) {
3870+
continue;
3871+
}
3872+
if (structKeyExists(this, local.varKey)) {
3873+
continue;
3874+
}
3875+
this[local.varKey] = variables[local.varKey];
3876+
}
3877+
38553878
}

vendor/wheels/WheelsTest.cfc

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,31 @@ component extends="wheels.wheelstest.system.BaseSpec" {
1010

1111
// Pseudo-constructor (runs automatically)
1212
if (structKeyExists(application, "wo")) {
13-
local.methods = getMetaData(application.wo).functions;
14-
15-
for (local.method in local.methods) {
16-
// Only add public, non-inherited methods
17-
if (local.method.access eq "public") {
18-
local.methodExists = structKeyExists(variables, local.method.name) || structKeyExists(this, local.method.name);
13+
// Iterate struct keys on application.wo and bind every UDF. This
14+
// catches both methods declared on Global.cfc (visible to
15+
// getMetaData) AND helpers merged in via cfinclude (e.g.
16+
// app/global/functions.cfm), which getMetaData(application.wo).functions
17+
// does NOT enumerate — see #2790.
18+
local.metaIndex = {};
19+
for (local.fn in getMetaData(application.wo).functions) {
20+
local.metaIndex[local.fn.name] = local.fn.access;
21+
}
1922

20-
if (!local.methodExists) {
21-
variables[local.method.name] = application.wo[local.method.name];
22-
this[local.method.name] = application.wo[local.method.name];
23-
}
23+
for (local.key in application.wo) {
24+
if (!isCustomFunction(application.wo[local.key])) {
25+
continue;
26+
}
27+
// For methods present in CFC metadata, keep the existing
28+
// public-only filter; include-injected helpers have no
29+
// access modifier so they're treated as public.
30+
if (structKeyExists(local.metaIndex, local.key) && local.metaIndex[local.key] neq "public") {
31+
continue;
32+
}
33+
if (structKeyExists(variables, local.key) || structKeyExists(this, local.key)) {
34+
continue;
2435
}
36+
variables[local.key] = application.wo[local.key];
37+
this[local.key] = application.wo[local.key];
2538
}
2639
}
2740

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* Regression: WheelsTest auto-bind misses include-injected helpers (#2790).
3+
*
4+
* `WheelsTest.cfc` uses `getMetaData(application.wo).functions` to discover
5+
* which Wheels globals to bind into the spec's `variables` / `this` scopes.
6+
* That metadata enumerates only methods defined directly on the CFC body,
7+
* NOT symbols merged in via `cfinclude` / `include` (which is how
8+
* `vendor/wheels/Global.cfc` pulls `/app/global/functions.cfm` at the bottom
9+
* of the file). User-defined global helpers therefore worked in controllers /
10+
* views / models but were invisible to test specs — every spec had to
11+
* manually rebind helpers in `beforeAll()`.
12+
*
13+
* Cross-engine note: a bare `include` inside a CFC body lands UDFs in the
14+
* component's `variables` scope, not `this`. Lucee's struct-iteration over
15+
* a CFC instance surfaces both scopes, but Adobe CF's contract only
16+
* reliably exposes `this`-scope members. To make the auto-bind path
17+
* uniform across engines, `vendor/wheels/Global.cfc` promotes include-
18+
* injected UDFs from `variables` to `this` immediately after the include
19+
* runs. These specs simulate that post-promotion shape by assigning the
20+
* probe UDF directly to `application.wo` (bracket-notation assignment
21+
* from outside writes to `this`), then assert that:
22+
*
23+
* - The probe is invisible to `getMetaData(application.wo).functions`
24+
* (the bug precondition the old code missed).
25+
* - The probe is enumerated by `for (key in application.wo)` — the
26+
* iteration mechanism the new auto-bind loop relies on. Failure here
27+
* on any engine means the auto-bind loop will silently miss the helper.
28+
* - The probe lands on a fresh `wheels.WheelsTest` instance and is
29+
* callable.
30+
*/
31+
component extends="wheels.WheelsTest" {
32+
33+
function run() {
34+
35+
describe("WheelsTest auto-bind", () => {
36+
37+
describe("helpers attached to application.wo outside of CFC metadata (issue ##2790)", () => {
38+
39+
it("the bug precondition holds: include-style UDFs are invisible to getMetaData", () => {
40+
var probeName = "$bot2790MetaProbe";
41+
application.wo[probeName] = function() {
42+
return "metadata-probe";
43+
};
44+
try {
45+
var meta = getMetaData(application.wo).functions;
46+
var foundInMeta = false;
47+
for (var fn in meta) {
48+
if (fn.name == probeName) {
49+
foundInMeta = true;
50+
break;
51+
}
52+
}
53+
expect(foundInMeta).toBeFalse();
54+
expect(structKeyExists(application.wo, probeName)).toBeTrue();
55+
expect(isCustomFunction(application.wo[probeName])).toBeTrue();
56+
} finally {
57+
structDelete(application.wo, probeName);
58+
}
59+
});
60+
61+
it("for-in iteration over application.wo enumerates the probe key", () => {
62+
// Guards the iteration mechanism the auto-bind loop in
63+
// WheelsTest.cfc relies on. If this fails on any engine
64+
// (notably Adobe CF, where struct-iteration over a CFC
65+
// only reliably exposes this-scope members), the bind
66+
// case below will silently pass-but-not-test.
67+
var probeName = "$bot2790IterProbe";
68+
application.wo[probeName] = function() {
69+
return "iter-probe";
70+
};
71+
try {
72+
var seen = false;
73+
for (var key in application.wo) {
74+
if (key == probeName) {
75+
seen = true;
76+
break;
77+
}
78+
}
79+
expect(seen).toBeTrue();
80+
} finally {
81+
structDelete(application.wo, probeName);
82+
}
83+
});
84+
85+
it("auto-binds include-style helpers into a fresh WheelsTest instance", () => {
86+
var probeName = "$bot2790BindProbe";
87+
application.wo[probeName] = function() {
88+
return "bind-probe";
89+
};
90+
try {
91+
var freshSpec = new wheels.WheelsTest();
92+
expect(structKeyExists(freshSpec, probeName)).toBeTrue();
93+
var bound = freshSpec[probeName];
94+
expect(bound()).toBe("bind-probe");
95+
} finally {
96+
structDelete(application.wo, probeName);
97+
}
98+
});
99+
100+
it("still binds methods that ARE in CFC metadata (regression guard for the existing path)", () => {
101+
var freshSpec = new wheels.WheelsTest();
102+
expect(structKeyExists(freshSpec, "model")).toBeTrue();
103+
expect(structKeyExists(freshSpec, "urlFor")).toBeTrue();
104+
});
105+
106+
});
107+
108+
});
109+
110+
}
111+
112+
}

0 commit comments

Comments
 (0)