diff --git a/frontend/src/app/main/data/viewer.cljs b/frontend/src/app/main/data/viewer.cljs index 40b99474ea..683bbcf8b9 100644 --- a/frontend/src/app/main/data/viewer.cljs +++ b/frontend/src/app/main/data/viewer.cljs @@ -368,9 +368,17 @@ ptk/WatchEvent (watch [_ state _] (let [zoom-type (get-in state [:viewer-local :zoom-type]) - params (rt/get-params state)] - - (rx/of (rt/nav :viewer (assoc params :zoom zoom-type))))))) + params (rt/get-params state) + current (rt/get-query-param params :zoom) + expected (some-> zoom-type name)] + ;; Zoom is view state: mirror it into the URL, replacing the history + ;; entry, only when the query string does not already describe it. + (when (not= current expected) + (rx/of (rt/nav :viewer + (if (some? zoom-type) + (assoc params :zoom zoom-type) + (dissoc params :zoom)) + {::rt/replace true}))))))) (def increase-zoom (ptk/reify ::increase-zoom diff --git a/frontend/src/app/main/data/workspace/interactions.cljs b/frontend/src/app/main/data/workspace/interactions.cljs index 2ef326cfbc..ccd7fa5c7c 100644 --- a/frontend/src/app/main/data/workspace/interactions.cljs +++ b/frontend/src/app/main/data/workspace/interactions.cljs @@ -210,7 +210,7 @@ ptk/WatchEvent (watch [_ _ _] (let [interactions (ctsi/update-interaction (:interactions shape) index update-fn) - interaction (nth interactions index)] + interaction (get interactions index)] (rx/of (dwsh/update-shapes [(:id shape)] diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index d42c0529a3..bbf3de8e1e 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -105,7 +105,8 @@ :get #(-> % u/proxy->interaction :event-type format/format-key) :set (fn [_ value] - (let [value (parser/parse-keyword value)] + (let [value (parser/parse-keyword value) + index (locate-index)] (cond (not (contains? ctsi/event-types value)) (u/not-valid plugin-id :trigger value) @@ -113,11 +114,14 @@ (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :trigger "Plugin doesn't have 'content:write' permission") + (nil? index) + (u/not-valid plugin-id :trigger "The interaction is not part of the shape anymore") + :else (do (st/emit! (dwi/update-interaction (u/locate-shape file-id page-id shape-id) - (locate-index) + index #(assoc % :event-type value) {:page-id page-id})) (swap! current assoc :event-type value)))))} @@ -127,21 +131,25 @@ :get #(-> % u/proxy->interaction :delay) :set (fn [_ value] - (cond - (or (not (sm/valid-safe-int? value)) (neg? value)) - (u/not-valid plugin-id :delay value) + (let [index (locate-index)] + (cond + (or (not (sm/valid-safe-int? value)) (neg? value)) + (u/not-valid plugin-id :delay value) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission") - :else - (do - (st/emit! (dwi/update-interaction - (u/locate-shape file-id page-id shape-id) - (locate-index) - #(assoc % :delay value) - {:page-id page-id})) - (swap! current assoc :delay value))))} + (nil? index) + (u/not-valid plugin-id :delay "The interaction is not part of the shape anymore") + + :else + (do + (st/emit! (dwi/update-interaction + (u/locate-shape file-id page-id shape-id) + index + #(assoc % :delay value) + {:page-id page-id})) + (swap! current assoc :delay value)))))} :action {:this true @@ -149,21 +157,27 @@ :set (fn [self value] (let [params (parser/parse-action value) + index (locate-index) interaction (-> (u/proxy->interaction self) (d/patch-object params))] (cond - (not (sm/validate ctsi/schema:interaction interaction)) - (u/not-valid plugin-id :action interaction) - (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :action "Plugin doesn't have 'content:write' permission") + ;; Precedes the schema check, which sees only the partial map that + ;; patching a missing interaction produces. + (nil? index) + (u/not-valid plugin-id :action "The interaction is not part of the shape anymore") + + (not (sm/validate ctsi/schema:interaction interaction)) + (u/not-valid plugin-id :action interaction) + :else (do (st/emit! (dwi/update-interaction (u/locate-shape file-id page-id shape-id) - (locate-index) + index #(d/patch-object % params) {:page-id page-id})) (reset! current interaction)))))} diff --git a/frontend/test/frontend_tests/data/viewer_test.cljs b/frontend/test/frontend_tests/data/viewer_test.cljs index bc0d35a998..482210b24d 100644 --- a/frontend/test/frontend_tests/data/viewer_test.cljs +++ b/frontend/test/frontend_tests/data/viewer_test.cljs @@ -8,6 +8,8 @@ (:require [app.common.uuid :as uuid] [app.main.data.viewer :as dv] + [app.main.router :as rt] + [beicon.v2.core :as rx] [cljs.test :as t] [potok.v2.core :as ptk])) @@ -67,3 +69,56 @@ result (ptk/update dv/zoom-to-fill state)] (t/is (= (get-in result [:viewer-local :zoom-type]) :fill)) (t/is (number? (get-in result [:viewer-local :zoom])))))) + +(defn- watch-events + "Collect the events an event's watch emits synchronously." + [event state] + (let [out (atom [])] + (some-> (ptk/watch event state nil) + (rx/subscribe #(swap! out conj %))) + @out)) + +(defn- zoom-state + "Build a viewer state with the given `:zoom` query param and zoom type." + [zoom-param zoom-type] + {:route {:params {:query (cond-> {:page-id (str page-id) :index "0"} + (some? zoom-param) + (assoc :zoom zoom-param))}} + :viewer-local (cond-> {} + (some? zoom-type) + (assoc :zoom-type zoom-type))}) + +(t/deftest update-zoom-querystring-does-not-navigate-when-url-already-matches + (t/testing "zoom type already described by the query string" + (t/is (empty? (watch-events dv/update-zoom-querystring + (zoom-state "fit" :fit))))) + + (t/testing "no zoom type and no zoom query param" + (t/is (empty? (watch-events dv/update-zoom-querystring + (zoom-state nil nil)))))) + +(t/deftest update-zoom-querystring-navigates-when-zoom-changes + (t/testing "zoom type differs from the query string" + (let [events (watch-events dv/update-zoom-querystring + (zoom-state "fit" :fill)) + {:keys [id params options]} (some-> (first events) deref)] + (t/is (= 1 (count events))) + (t/is (= :viewer id)) + (t/is (= :fill (:zoom params))) + (t/is (true? (::rt/replace options))))) + + (t/testing "zoom query param absent, other params preserved" + (let [events (watch-events dv/update-zoom-querystring + (zoom-state nil :fit)) + {:keys [params]} (some-> (first events) deref)] + (t/is (= 1 (count events))) + (t/is (= :fit (:zoom params))) + (t/is (= (str page-id) (:page-id params))) + (t/is (= "0" (:index params))))) + + (t/testing "zoom type cleared drops the query param" + (let [events (watch-events dv/update-zoom-querystring + (zoom-state "fit" nil)) + {:keys [params]} (some-> (first events) deref)] + (t/is (= 1 (count events))) + (t/is (not (contains? params :zoom)))))) diff --git a/mcp/README.md b/mcp/README.md index 702eec3d60..3189a4dea3 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -265,7 +265,8 @@ The Penpot MCP server can be configured using environment variables. | `PENPOT_MCP_SERVER_PORT` | Port for the HTTP/SSE server | `4401` | | `PENPOT_MCP_WEBSOCKET_PORT` | Port for the WebSocket server (plugin connection) | `4402` | | `PENPOT_MCP_REPL_PORT` | Port for the REPL server (development/debugging) | `4403` | -| `PENPOT_MCP_REPL_ENABLE` | Explicitly enable/disable the REPL server. Set to `true` to enable. When unset, defaults to the value of `PENPOT_MCP_DEVENV`. | (unset) | +| `PENPOT_MCP_REPL_HOST` | Address on which the REPL server listens (binds to) | `localhost` | +| `PENPOT_MCP_REPL_ENABLE` | Explicitly enable/disable the REPL server. Set to `true` to enable. When unset, defaults to the value of `PENPOT_MCP_DEVENV`. The REPL server never starts in multi-user mode. | (unset) | | `PENPOT_MCP_REMOTE_MODE` | Enable remote mode (disables file system access). Set to `true` to enable. | `false` | | `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools in local single-user mode. Set to `true` to enable. | `false` | | `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` | diff --git a/mcp/packages/server/src/PenpotMcpServer.test.ts b/mcp/packages/server/src/PenpotMcpServer.test.ts index 68665359f5..2d35e70589 100644 --- a/mcp/packages/server/src/PenpotMcpServer.test.ts +++ b/mcp/packages/server/src/PenpotMcpServer.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { PenpotMcpServer, shouldRegisterDeveloperTools } from "./PenpotMcpServer"; +import { PenpotMcpServer, shouldRegisterDeveloperTools, shouldStartReplServer } from "./PenpotMcpServer"; test("registers developer tools in local devenv mode", () => { assert.equal(shouldRegisterDeveloperTools(true, false), true); @@ -14,6 +14,22 @@ test("does not register developer tools when devenv mode is disabled", () => { assert.equal(shouldRegisterDeveloperTools(false, false), false); }); +test("starts REPL server in single-user mode when enabled", () => { + assert.equal(shouldStartReplServer(true, false), true); +}); + +test("does not start REPL server in multi-user mode even when enabled", () => { + assert.equal(shouldStartReplServer(true, true), false); +}); + +test("does not start REPL server when disabled in single-user mode", () => { + assert.equal(shouldStartReplServer(false, false), false); +}); + +test("does not start REPL server when disabled in multi-user mode", () => { + assert.equal(shouldStartReplServer(false, true), false); +}); + // ── Pure function tests ──────────────────────────────────────── test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is not set", () => { @@ -125,6 +141,79 @@ test("constructor does not create ReplServer when PENPOT_MCP_REPL_ENABLE is 'fal } }); +test("constructor does not create ReplServer in multi-user mode even with DEVENV", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + delete process.env.PENPOT_MCP_REPL_ENABLE; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(true); + assert.equal(server.hasReplServer(), false); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable); + } +}); + +test("constructor does not create ReplServer in multi-user mode even with explicit REPL_ENABLE", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE; + const prevPorts = setUniqueEnv(); + delete process.env.PENPOT_MCP_DEVENV; + process.env.PENPOT_MCP_REPL_ENABLE = "true"; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(true); + assert.equal(server.hasReplServer(), false); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable); + } +}); + +test("replHost defaults to localhost and ignores SERVER_HOST", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevServerHost = process.env.PENPOT_MCP_SERVER_HOST; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + process.env.PENPOT_MCP_SERVER_HOST = "0.0.0.0"; + delete process.env.PENPOT_MCP_REPL_HOST; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), true); + assert.equal(server.replHost, "localhost"); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_SERVER_HOST", prevServerHost); + restoreOrDelete("PENPOT_MCP_REPL_HOST", prevReplHost); + } +}); + +test("replHost respects PENPOT_MCP_REPL_HOST", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + process.env.PENPOT_MCP_REPL_HOST = "0.0.0.0"; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), true); + assert.equal(server.replHost, "0.0.0.0"); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_HOST", prevReplHost); + } +}); + // ── Helpers ──────────────────────────────────────────────────── function setUniqueEnv() { @@ -132,15 +221,22 @@ function setUniqueEnv() { const prevServer = process.env.PENPOT_MCP_SERVER_PORT; const prevWs = process.env.PENPOT_MCP_WEBSOCKET_PORT; const prevRepl = process.env.PENPOT_MCP_REPL_PORT; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; process.env.PENPOT_MCP_SERVER_PORT = String(ports.server); process.env.PENPOT_MCP_WEBSOCKET_PORT = String(ports.ws); process.env.PENPOT_MCP_REPL_PORT = String(ports.repl); - return { prevServer, prevWs, prevRepl }; + delete process.env.PENPOT_MCP_REPL_HOST; + return { prevServer, prevWs, prevRepl, prevReplHost }; } function restoreEnv( devEnv: string | undefined, - ports: { prevServer: string | undefined; prevWs: string | undefined; prevRepl: string | undefined } + ports: { + prevServer: string | undefined; + prevWs: string | undefined; + prevRepl: string | undefined; + prevReplHost: string | undefined; + } ) { if (devEnv !== undefined) { process.env.PENPOT_MCP_DEVENV = devEnv; @@ -150,6 +246,7 @@ function restoreEnv( restoreOrDelete("PENPOT_MCP_SERVER_PORT", ports.prevServer); restoreOrDelete("PENPOT_MCP_WEBSOCKET_PORT", ports.prevWs); restoreOrDelete("PENPOT_MCP_REPL_PORT", ports.prevRepl); + restoreOrDelete("PENPOT_MCP_REPL_HOST", ports.prevReplHost); } function restoreOrDelete(key: string, value: string | undefined) { diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index c620aca3fc..4691c0541d 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -57,6 +57,16 @@ export function shouldRegisterDeveloperTools(isDevEnv: boolean, isMultiUserMode: return isDevEnv && !isMultiUserMode; } +/** + * Indicates whether the REPL server may be started for the current server mode. + * + * The REPL server never starts in multi-user mode, even when explicitly + * enabled, mirroring the developer tools policy. + */ +export function shouldStartReplServer(isReplEnabled: boolean, isMultiUserMode: boolean): boolean { + return isReplEnabled && !isMultiUserMode; +} + export class PenpotMcpServer { /** * Timeout, in minutes, for idle sessions (Streamable HTTP and SSE) before they are automatically closed and removed. @@ -133,6 +143,7 @@ export class PenpotMcpServer { public readonly host: string; public readonly port: number; public readonly webSocketPort: number; + public readonly replHost: string; public readonly replPort: number; private sessionTimeoutInterval: ReturnType | undefined; @@ -156,6 +167,7 @@ export class PenpotMcpServer { this.host = process.env.PENPOT_MCP_SERVER_HOST ?? "localhost"; this.port = parseInt(process.env.PENPOT_MCP_SERVER_PORT ?? "4401", 10); this.webSocketPort = parseInt(process.env.PENPOT_MCP_WEBSOCKET_PORT ?? "4402", 10); + this.replHost = process.env.PENPOT_MCP_REPL_HOST ?? "localhost"; this.replPort = parseInt(process.env.PENPOT_MCP_REPL_PORT ?? "4403", 10); this.tenant = process.env.PENPOT_TENANT ?? "default"; const toolTimeoutSecs = parseInt(process.env.PENPOT_MCP_TOOL_TIMEOUT_S ?? "120", 10); @@ -181,8 +193,8 @@ export class PenpotMcpServer { this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge); - if (PenpotMcpServer.isReplEnabled(process.env)) { - this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); + if (shouldStartReplServer(PenpotMcpServer.isReplEnabled(process.env), this.isMultiUserMode())) { + this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.replHost); } else { this.replServer = null; } @@ -232,9 +244,10 @@ export class PenpotMcpServer { /** * Indicates whether the REPL server was created. * - * The REPL server is created when {@link isReplEnabled} returns true, - * which means either ``PENPOT_MCP_REPL_ENABLE=true`` or, when that - * variable is unset, ``PENPOT_MCP_DEVENV=true``. + * The REPL server is created when {@link isReplEnabled} returns true and + * the server is not running in multi-user mode, which means either + * ``PENPOT_MCP_REPL_ENABLE=true`` or, when that variable is unset, + * ``PENPOT_MCP_DEVENV=true``, in single-user mode. */ public hasReplServer(): boolean { return this.replServer !== null; @@ -471,6 +484,8 @@ export class PenpotMcpServer { // start the REPL server (devenv only) and session timeout checker if (this.replServer) { await this.replServer.start(); + } else if (this.isMultiUserMode()) { + this.logger.info("REPL server disabled in multi-user mode (never started with --multi-user)"); } else { this.logger.info( "REPL server disabled (set PENPOT_MCP_REPL_ENABLE=true or PENPOT_MCP_DEVENV=true to enable)" diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index 6a1e07f9c5..305f5dc35e 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -94,7 +94,7 @@ export class PluginBridge { private readonly taskTimeoutSecs: number, private readonly redisBridge?: RedisBridge ) { - this.wsServer = new WebSocketServer({ port: port }); + this.wsServer = new WebSocketServer({ port: port, host: mcpServer.host }); this.setupWebSocketHandlers(); } diff --git a/mcp/scripts/start-mcp-devenv b/mcp/scripts/start-mcp-devenv index 62e957a9eb..9098a8b254 100755 --- a/mcp/scripts/start-mcp-devenv +++ b/mcp/scripts/start-mcp-devenv @@ -3,4 +3,4 @@ # This starts the MCP server in a configuration for Penpot development # (assuming devenv) -PENPOT_MCP_SERVER_HOST=0.0.0.0 PENPOT_MCP_REMOTE_MODE=true PENPOT_MCP_DEVENV=true pnpm run bootstrap +PENPOT_MCP_SERVER_HOST=0.0.0.0 PENPOT_MCP_REPL_HOST=0.0.0.0 PENPOT_MCP_REMOTE_MODE=true PENPOT_MCP_DEVENV=true pnpm run bootstrap diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index e4e577f263..35edd55869 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -10,6 +10,7 @@ - **plugins-runtime**: An interaction obtained from `Shape.interactions` now keeps addressing that interaction instead of the position it held when the array was read. Removing every interaction of a shape from a single read removes all of them rather than leaving some behind, and writing through a held interaction after an earlier one is removed no longer lands on a different interaction. - **plugins-runtime**: `Shape.removeInteraction()` now rejects an interaction belonging to a different shape with a validation error, instead of removing whichever interaction sat at the same position on the target shape. +- **plugins-runtime**: Writing `trigger`, `delay` or `action` on an interaction the shape no longer has now raises a validation error. The write used to be sent to the workspace with no position to apply it at, where it failed out of the plugin's reach: nothing was written and nothing was reported. - **plugins-runtime**: `Library.createComponent()` now rejects invalid input (an empty shape list, or a shape inside a component copy) with a validation error instead of returning a component proxy pointing at nothing. - **plugins-runtime**: Setting an individual padding/margin side (`leftPadding`, `topMargin`, …) now re-derives the padding/margin type, switching to `multiple` when the four sides stop being symmetric (so the value is actually painted) and back to `simple` once top/bottom and left/right are mirrored again. - **plugins-runtime**: Removed the premature deep-hardening of the host plugin context, which froze shared host functions (including `Function.prototype`) before SES override taming, causing `TypeError: Cannot assign to read only property 'toString'` on later host-side function extension. Related to #11001. diff --git a/plugins/apps/plugin-api-test-suite/README.md b/plugins/apps/plugin-api-test-suite/README.md index af01f7e4ca..5ea524b484 100644 --- a/plugins/apps/plugin-api-test-suite/README.md +++ b/plugins/apps/plugin-api-test-suite/README.md @@ -69,6 +69,27 @@ E2E_LOGIN_EMAIL=… E2E_LOGIN_PASSWORD=… \ - `PRINT_UNCOVERED=1` dumps the uncovered targets per interface; `PRINT_STATIC=1` dumps the statically-covered ones (see [Coverage](#how-coverage-works-and-how-to-write-tests-that-move-it)). +### Errors the app reports on its own + +An API call can leave the plugin happy and still break Penpot: an exception +raised inside an event handler is caught by the store, which reports it and +carries on, so nothing is thrown back across the sandbox. The CI runner watches +the page console for the prefixes Penpot's error handler prints (`Internal +Error`, `Unexpected Error`, `Assertion Error`, `Uncaught Exception`, `Uncaught +Rejection`) and for uncaught page errors, and fails the test that was running. + +Two consequences when writing a test for such a case: + +- Await the API (`await ctx.penpot.waitForLayoutUpdate()`) after the operation, + so the message reaches the console before the test ends and is attributed to + it rather than to the next one. +- This runs in the CI runner only. The plugin UI cannot read the page console, + so the same test shows green there — check it with `test:ci` or + `test:ci:mocked`. + +`Plugin Error` and `Network Error` are not watched: tests provoke both on +purpose. + CI entry points reuse the exact same test files (`src/ci/headless.ts` discovers them the same way the plugin does). diff --git a/plugins/apps/plugin-api-test-suite/ci/run-ci.ts b/plugins/apps/plugin-api-test-suite/ci/run-ci.ts index 801d62779d..559e6f0aea 100644 --- a/plugins/apps/plugin-api-test-suite/ci/run-ci.ts +++ b/plugins/apps/plugin-api-test-suite/ci/run-ci.ts @@ -34,6 +34,13 @@ const frontendDir = resolve(repoRoot, 'frontend'); const staticRoot = resolve(frontendDir, 'resources/public'); const e2eDataDir = resolve(frontendDir, 'playwright/data'); +// Console prefixes Penpot's error handler prints for failures the app did not +// expect (`frontend/src/app/main/errors.cljs`). The store swallows these, so +// the console is the only place a test can observe them. "Plugin Error" and +// "Network Error" are left out: tests provoke both on purpose. +const APP_ERROR_RE = + /^(Internal Error|Unexpected Error|Assertion Error|Uncaught Exception|Uncaught Rejection):/; + const MOCKED = !!process.env['MOCK_BACKEND']; const MOCK_BASE_URL = 'http://localhost:3000'; const apiUrl = MOCKED @@ -393,13 +400,36 @@ async function main() { let fatal: string | null = null; console.log('\nRunning tests:'); + // Errors the app reported since the previous test result. + let appErrors: string[] = []; + const takeAppErrors = (): string => { + const detail = appErrors.join('; '); + appErrors = []; + return detail; + }; + const done = new Promise((resolvePromise) => { + page.on('pageerror', (err) => { + appErrors.push(`Uncaught ${err.message}`); + }); page.on('console', (msg) => { const text = msg.text(); + if (APP_ERROR_RE.test(text)) { + appErrors.push(text.split('\n')[0]!.trim()); + } if (text.startsWith('__TEST_RESULT__ ')) { const result: TestResult = JSON.parse( text.slice('__TEST_RESULT__ '.length), ); + // Errors buffered so far belong to the test this result closes. + const reported = takeAppErrors(); + if (reported) { + result.error = + result.status === 'fail' && result.error + ? `${result.error} — Penpot also reported: ${reported}` + : `Penpot reported an error during the test: ${reported}`; + result.status = 'fail'; + } results.push(result); // Print each result as it streams in so the run shows live progress // instead of staying silent until it finishes. diff --git a/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts index 6f01f8ef0f..a7fcc88051 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts @@ -413,6 +413,28 @@ describe('Interactions', () => { expect(r.interactions.map((i) => i.delay)).toEqual([null, 500]); }); + // A proxy whose own interaction is gone resolves to no position, so the write + // has nothing to address and the API rejects it. + test('a write through a removed interaction is rejected', async (ctx) => { + const r = rect(ctx); + r.addInteraction('click', { + type: 'open-url', + url: 'https://example.com', + }); + await ctx.penpot.waitForLayoutUpdate(); + const [only] = r.interactions; + + only.remove(); + await ctx.penpot.waitForLayoutUpdate(); + expect(r.interactions).toHaveLength(0); + + expect(() => { + only.delay = 500; + }).toThrow('The interaction is not part of the shape anymore'); + await ctx.penpot.waitForLayoutUpdate(); + expect(r.interactions).toHaveLength(0); + }); + test('interaction trigger can be changed', (ctx) => { const dest = board(ctx); const r = rect(ctx);