Skip to content

Commit 75fc2f7

Browse files
zaloclaude
andcommitted
Fix PR review comments, update CI, and add Playwright test suite
Address all Copilot review comments across PRs 169-171: - Fix worker requestId response guard (always respond when requestId present) - Fix MessageBus falsy response handling and add request timeout - Fix CascadeAPI duplicate meta tag and evaluate() concurrent call safety - Fix ConsoleManager + worker multi-arg console.log capture - Clear console on each evaluation in EditorManager - Fix OpenSCAD transpiler: cylinder defaults, square dynamic centering, for/range negative step handling, multi-child transform shape removal - Fix Mirror bug (shapes vs shapes[shapeIndex]) - Pass fuzzValue to BRepAlgoAPI boolean operations - Fix argCache reference-breaking reassignment in ShapeToMesh and Text3D - Add sceneOptions undefined guard in CascadeView - Fix String.fromCharCode stack overflow for large projects - Fix duplicate event listeners on re-initialization - Use execFileSync instead of execSync to avoid shell injection Update CI from deprecated GitHub Actions v2 to v4 with Playwright. Add comprehensive Playwright test suite (42 tests covering startup, API, primitives, transforms, booleans, operations, export, GUI, console). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2ff9047 commit 75fc2f7

21 files changed

Lines changed: 1420 additions & 120 deletions

.github/workflows/ci.yml

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,48 +2,42 @@ name: CascadeStudio - End To End Test
22

33
on:
44
push:
5+
pull_request:
56

67
jobs:
78
test:
89
name: End to End Test
9-
runs-on: windows-latest
10+
runs-on: ubuntu-latest
1011
steps:
1112
- name: Checkout Repository
12-
uses: actions/checkout@v2
13+
uses: actions/checkout@v4
1314
with:
1415
lfs: true
1516

16-
- name: Cache Node Modules
17-
uses: actions/cache@v2
18-
env:
19-
cache-name: cache-node-modules
17+
- name: Setup Node.js
18+
uses: actions/setup-node@v4
2019
with:
21-
# npm cache files are stored in `~/.npm` on Linux/macOS
22-
path: ~/.npm
23-
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
24-
restore-keys: |
25-
${{ runner.os }}-build-${{ env.cache-name }}-
26-
${{ runner.os }}-build-
27-
${{ runner.os }}-
28-
29-
- name: Install Testing Node Modules
30-
run: |
31-
cd test
32-
npm install
33-
34-
- name: Run Server and Tests
35-
run: |
36-
start python "test/server.py"
37-
cd test
38-
npx folio --param screenshotOnFailure --param browserName=chromium
39-
40-
- name: Upload Failing Screenshots
41-
uses: actions/upload-artifact@v2
42-
if: failure()
43-
with:
44-
name: Screenshots
45-
path: test/test-results
20+
node-version: 20
21+
cache: 'npm'
22+
23+
- name: Install Dependencies
24+
run: npm ci
25+
26+
- name: Install Playwright Browsers
27+
run: npx playwright install --with-deps chromium
4628

47-
- name: Where to go from here
29+
- name: Build Project
30+
run: npm run build
31+
32+
- name: Run Tests
33+
run: npx playwright test
34+
35+
- name: Upload Test Results
36+
uses: actions/upload-artifact@v4
4837
if: failure()
49-
run: echo "~~~*** Test failed! See the README.md in CascadeStudio/test for more information ***~~~"
38+
with:
39+
name: playwright-report
40+
path: |
41+
playwright-report/
42+
test-results/
43+
retention-days: 7

js/CADWorker/CascadeStudioMainWorker.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,10 @@ class CascadeStudioWorker {
4141
const realLog = this.realConsoleLog;
4242
const realError = this.realConsoleError;
4343

44-
console.log = function (message) {
44+
console.log = function (...args) {
45+
const message = args.map(a => typeof a === 'string' ? a : JSON.stringify(a)).join(' ');
4546
setTimeout(() => { postMessage({ type: "log", payload: message }); }, 0);
46-
realLog.apply(console, arguments);
47+
realLog.apply(console, args);
4748
};
4849

4950
console.error = function (err, url, line, colno, errorObj) {
@@ -116,7 +117,7 @@ class CascadeStudioWorker {
116117
// Route incoming messages to registered handlers
117118
onmessage = function (e) {
118119
let response = self.messageHandlers[e.data.type](e.data.payload);
119-
if (response) {
120+
if (response !== undefined || e.data.requestId) {
120121
const msg = { "type": e.data.type, payload: response };
121122
if (e.data.requestId) { msg.requestId = e.data.requestId; }
122123
postMessage(msg);

js/CADWorker/CascadeStudioShapeToMesh.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class CascadeStudioMesher {
6969
CascadeStudioMesher.forEachFace(shape, (faceIndex, myFace) => {
7070
let aLocation = new oc.TopLoc_Location_1();
7171
let myT = oc.BRep_Tool.Triangulation(myFace, aLocation, 0 /* Poly_MeshPurpose_NONE */);
72-
if (myT.IsNull()) { console.error("Encountered Null Face!"); self.argCache = {}; return; }
72+
if (myT.IsNull()) { console.error("Encountered Null Face!"); for (let k in self.argCache) delete self.argCache[k]; return; }
7373

7474
let this_face = {
7575
vertex_coord: [],

js/CADWorker/CascadeStudioStandardLibrary.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function Text3D(text, size, height, fontName) {
123123

124124
let textArgs = JSON.stringify(arguments);
125125
let curText = self.CacheOp(arguments, "Text3D", () => {
126-
if (self.loadedFonts[fontName] === undefined) { self.argCache = {}; console.log("Font not loaded or found yet! Try again..."); return; }
126+
if (self.loadedFonts[fontName] === undefined) { for (let k in self.argCache) delete self.argCache[k]; console.log("Font not loaded or found yet! Try again..."); return; }
127127
let textFaces = [];
128128
let commands = self.loadedFonts[fontName].getPath(text, 0, 0, size).commands;
129129
for (let idx = 0; idx < commands.length; idx++) {
@@ -413,7 +413,7 @@ function Mirror(vector, shapes, keepOriginal) {
413413
} else if (shapes.length >= 1) {
414414
let newMirroring = [];
415415
for (let shapeIndex = 0; shapeIndex < shapes.length; shapeIndex++) {
416-
newMirroring.push(new self.oc.BRepBuilderAPI_Transform_2(shapes, mirrorTransform, false).Shape());
416+
newMirroring.push(new self.oc.BRepBuilderAPI_Transform_2(shapes[shapeIndex], mirrorTransform, false).Shape());
417417
}
418418
return newMirroring;
419419
}
@@ -456,6 +456,8 @@ function Union(objectsToJoin, keepObjects, fuzzValue, keepEdges) {
456456
for (let i = 0; i < objectsToJoin.length; i++) {
457457
if (i > 0) {
458458
let combinedFuse = new self.oc.BRepAlgoAPI_Fuse_3(combined, objectsToJoin[i], new self.oc.Message_ProgressRange_1());
459+
combinedFuse.SetFuzzyValue(fuzzValue);
460+
combinedFuse.Build(new self.oc.Message_ProgressRange_1());
459461
combined = combinedFuse.Shape();
460462
}
461463
}
@@ -486,6 +488,8 @@ function Difference(mainBody, objectsToSubtract, keepObjects, fuzzValue, keepEdg
486488
for (let i = 0; i < objectsToSubtract.length; i++) {
487489
if (!objectsToSubtract[i] || objectsToSubtract[i].IsNull()) { console.error("Tool in Difference is null!"); }
488490
let differenceCut = new self.oc.BRepAlgoAPI_Cut_3(difference, objectsToSubtract[i], new self.oc.Message_ProgressRange_1());
491+
differenceCut.SetFuzzyValue(fuzzValue);
492+
differenceCut.Build(new self.oc.Message_ProgressRange_1());
489493
difference = differenceCut.Shape();
490494
}
491495
}
@@ -527,6 +531,8 @@ function Intersection(objectsToIntersect, keepObjects, fuzzValue, keepEdges) {
527531
for (let i = 0; i < objectsToIntersect.length; i++) {
528532
if (i > 0) {
529533
let intersectedCommon = new self.oc.BRepAlgoAPI_Common_3(intersected, objectsToIntersect[i], new self.oc.Message_ProgressRange_1());
534+
intersectedCommon.SetFuzzyValue(fuzzValue);
535+
intersectedCommon.Build(new self.oc.Message_ProgressRange_1());
530536
intersected = intersectedCommon.Shape();
531537
}
532538
}

js/MainPage/CascadeAPI.js

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ class CascadeAPI {
1010
this._app = app;
1111
}
1212

13-
/** Install the API on window and add discovery meta tag. */
13+
/** Install the API on window and update discovery meta tag. */
1414
install() {
1515
window.CascadeAPI = this;
16-
// Add meta tag for agent discovery
17-
const meta = document.createElement('meta');
18-
meta.name = 'cascade-api';
16+
// Update existing meta tag or create one for agent discovery
17+
let meta = document.querySelector('meta[name="cascade-api"]');
18+
if (!meta) {
19+
meta = document.createElement('meta');
20+
meta.name = 'cascade-api';
21+
document.head.appendChild(meta);
22+
}
1923
meta.content = 'window.CascadeAPI';
20-
document.head.appendChild(meta);
2124
}
2225

2326
// --- Code Management ---
@@ -34,17 +37,25 @@ class CascadeAPI {
3437

3538
/** Evaluate the current code and return a Promise that resolves when generation is complete. */
3639
evaluate() {
37-
return new Promise((resolve) => {
38-
// Listen for generation completion
40+
if (this._evaluatePromise) { return this._evaluatePromise; }
41+
this._evaluatePromise = new Promise((resolve) => {
42+
// Wrap the existing handler to detect completion
3943
const originalHandler = this._app.messageBus.handlers["combineAndRenderShapes"];
4044
this._app.messageBus.on("combineAndRenderShapes", (payload) => {
41-
// Restore original handler and call it
42-
this._app.messageBus.on("combineAndRenderShapes", originalHandler);
45+
// Restore original handler first
46+
if (originalHandler) {
47+
this._app.messageBus.on("combineAndRenderShapes", originalHandler);
48+
} else {
49+
this._app.messageBus.off("combineAndRenderShapes");
50+
}
51+
// Call original handler
4352
if (originalHandler) originalHandler(payload);
53+
this._evaluatePromise = null;
4454
resolve();
4555
});
4656
this._app.editor.evaluateCode(false);
4757
});
58+
return this._evaluatePromise;
4859
}
4960

5061
// --- Results ---

js/MainPage/CascadeMain.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,18 @@ class CascadeStudioApp {
141141
});
142142
}
143143

144-
// Resize the layout when the browser resizes
145-
const updateLayoutSize = () => {
144+
// Resize the layout when the browser resizes (remove old listeners first)
145+
if (this._updateLayoutSize) {
146+
window.removeEventListener('resize', this._updateLayoutSize);
147+
window.removeEventListener('orientationchange', this._updateLayoutSize);
148+
}
149+
this._updateLayoutSize = () => {
146150
const h = window.innerHeight - document.getElementById('topnav').offsetHeight;
147151
this.myLayout.setSize(window.innerWidth, h);
148152
};
149-
window.addEventListener('resize', updateLayoutSize);
150-
window.addEventListener('orientationchange', updateLayoutSize);
151-
requestAnimationFrame(updateLayoutSize);
153+
window.addEventListener('resize', this._updateLayoutSize);
154+
window.addEventListener('orientationchange', this._updateLayoutSize);
155+
requestAnimationFrame(this._updateLayoutSize);
152156

153157
// Register startup callback from the CAD Worker
154158
this.messageBus.on("startupCallback", () => {
@@ -293,7 +297,12 @@ class CascadeStudioApp {
293297
/** Encode a string to a base64+compressed version. Uses fflate. */
294298
static encode(string) {
295299
const compressed = deflateSync(strToU8(string));
296-
const binaryStr = String.fromCharCode(...compressed);
300+
// Build binary string in chunks to avoid stack overflow with large data
301+
let binaryStr = '';
302+
const chunkSize = 8192;
303+
for (let i = 0; i < compressed.length; i += chunkSize) {
304+
binaryStr += String.fromCharCode.apply(null, compressed.subarray(i, i + chunkSize));
305+
}
297306
return encodeURIComponent(btoa(binaryStr));
298307
}
299308

js/MainPage/CascadeView.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ class CascadeEnvironment {
147147
messageBus.on("combineAndRenderShapes", ([[facelist, edgelist], sceneOptions]) => {
148148
window.workerWorking = false;
149149
if (!facelist) { return; }
150+
if (!sceneOptions) { sceneOptions = {}; }
150151

151152
// The old mainObject is dead! Long live the mainObject!
152153
this.environment.scene.remove(this.mainObject);

js/MainPage/ConsoleManager.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,17 @@ class ConsoleManager {
9797
this._realConsoleLog = console.log;
9898
const self = this;
9999

100-
console.log = function (message) {
100+
console.log = function (...args) {
101101
let messageText;
102-
if (message !== undefined) {
103-
messageText = JSON.stringify(message, ConsoleManager._circularReplacer());
104-
if (messageText.startsWith('"')) { messageText = messageText.slice(1, -1); }
102+
if (args.length === 0) {
103+
messageText = "";
105104
} else {
106-
messageText = "undefined";
105+
messageText = args.map(arg => {
106+
if (arg === undefined) return "undefined";
107+
let s = JSON.stringify(arg, ConsoleManager._circularReplacer());
108+
if (s && s.startsWith('"')) { s = s.slice(1, -1); }
109+
return s;
110+
}).join(' ');
107111
}
108112

109113
self.logs.push(messageText);
@@ -115,7 +119,7 @@ class ConsoleManager {
115119
newline.innerHTML = "&gt; " + messageText;
116120
self._consoleContainer.appendChild(newline);
117121
self._consoleContainer.parentElement.scrollTop = self._consoleContainer.parentElement.scrollHeight;
118-
self._realConsoleLog.apply(console, arguments);
122+
self._realConsoleLog.apply(console, args);
119123
};
120124
}
121125

js/MainPage/EditorManager.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ class EditorManager {
9797
let newCode = this.editor.getValue();
9898
monaco.editor.setModelMarkers(this.editor.getModel(), 'test', []);
9999

100-
// Refresh the GUI Panel
100+
// Clear console and refresh the GUI Panel
101+
this._app.console.clear();
101102
this._app.gui.reset();
102103
this._app.viewport.clearTransformHandles();
103104

js/MainPage/MessageBus.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class MessageBus {
2323
// Route to registered handler
2424
if (type in this._handlers) {
2525
let response = this._handlers[type](payload);
26-
if (response) {
26+
if (response !== undefined) {
2727
this._worker.postMessage({ type, payload: response });
2828
}
2929
}
@@ -50,11 +50,18 @@ class MessageBus {
5050
}
5151

5252
/** Send a message and return a Promise that resolves with the response.
53-
* Requires the worker to propagate requestId back. */
54-
request(type, payload) {
53+
* Requires the worker to propagate requestId back. Times out after 30s. */
54+
request(type, payload, timeoutMs = 30000) {
5555
const requestId = this._nextRequestId++;
56-
return new Promise((resolve) => {
57-
this._pendingRequests[requestId] = resolve;
56+
return new Promise((resolve, reject) => {
57+
const timer = setTimeout(() => {
58+
delete this._pendingRequests[requestId];
59+
reject(new Error(`MessageBus request '${type}' timed out after ${timeoutMs}ms`));
60+
}, timeoutMs);
61+
this._pendingRequests[requestId] = (result) => {
62+
clearTimeout(timer);
63+
resolve(result);
64+
};
5865
this._worker.postMessage({ type, payload, requestId });
5966
});
6067
}

0 commit comments

Comments
 (0)