diff --git a/.github/workflows/ci-test.yaml b/.github/workflows/ci-test.yaml index b0fe661ec1..858022df0f 100644 --- a/.github/workflows/ci-test.yaml +++ b/.github/workflows/ci-test.yaml @@ -25,16 +25,18 @@ jobs: # skipped via `if:` reports conclusion `skipped`, which passes. Two scopes: # `relevant` gates unit/integration; `e2e_relevant` gates e2e and skips only # docs — the e2e image build is the sole CI check that compiles the frontend, - # and docker/** changes must be e2e-tested. + # and docker/** changes must be e2e-tested. `frontend` gates the JS tiers, + # which are the mirror image of `relevant`: only frontend/** matters to them. changes: runs-on: ubuntu-latest outputs: # Filtering is a PR-time saving only. On workflow_dispatch there is no # base to diff, so paths-filter resolves against the default branch and a # manual run on main diffs main against itself — every filter false, every - # tier skipped, gate green with nothing run. Force both true off-PR. + # tier skipped, gate green with nothing run. Force all true off-PR. relevant: ${{ github.event_name != 'pull_request' || steps.filter.outputs.relevant == 'true' }} e2e_relevant: ${{ github.event_name != 'pull_request' || steps.filter.outputs.e2e_relevant == 'true' }} + frontend: ${{ github.event_name != 'pull_request' || steps.filter.outputs.frontend == 'true' }} steps: - name: Checkout repository uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -54,6 +56,8 @@ jobs: - "!docker/**" - "!frontend/**" - "!docs/**" + frontend: + - "frontend/**" e2e_relevant: - "!docs/**" @@ -124,6 +128,78 @@ jobs: if-no-files-found: ignore retention-days: 14 + # Frontend tiers. Runs alongside the python tiers, not after them: nothing is + # shared, and the whole job is minutes against e2e's tens of minutes. + # + # Two vitest projects, one job: + # unit — happy-dom. Fast, but every rect measures 0, so it cannot see + # layout at all. + # layout — real Chromium. Asserts the geometric contracts of list rows + # (columns line up, nothing spills onto the action icons, nothing + # overflows) at the widths the CSS claims to support. These are + # the regressions that reach staging because a CSS diff looks + # innocent in review. + frontend: + needs: changes + if: needs.changes.outputs.frontend == 'true' && github.event.pull_request.draft == false + runs-on: ubuntu-latest + timeout-minutes: 15 + defaults: + run: + working-directory: frontend + steps: + - name: Checkout repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Set up bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 # v2.0.2 + with: + bun-version: "1.3.11" + + - name: Install dependencies + # --ignore-scripts matches the frontend image build. Playwright's + # postinstall browser download is one of the scripts it skips; the + # dedicated step below does that explicitly and cacheably instead. + run: bun install --frozen-lockfile --ignore-scripts + + # Keyed on the lockfile: the browser build is pinned by the playwright + # version in it, so a bump invalidates the cache and re-downloads. + - name: Cache Playwright browsers + id: playwright-cache + uses: actions/cache@v5 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('frontend/bun.lock') }} + restore-keys: | + ${{ runner.os }}-playwright- + + - name: Install Chromium + # The locally installed binary, not `bunx playwright`: that would fetch + # whatever version the registry serves, while this is the one pinned in + # bun.lock and matched to @vitest/browser. + # --with-deps installs the shared libraries headless Chromium needs; + # they live outside the cached browser dir, so this runs either way. + run: ./node_modules/.bin/playwright install chromium --with-deps + + - name: Run unit tests + run: bun run test:unit + + - name: Run layout tests + run: bun run test:layout + + - name: Upload layout failure screenshots + # Written by vitest's browser mode on failure — a red layout assertion + # is far quicker to read as a picture than as coordinates. + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: frontend-layout-screenshots + path: frontend/src/**/__screenshots__/** + if-no-files-found: ignore + retention-days: 14 + # Builds images then runs e2e against the full compose platform. Gated by # `e2e_relevant`, which unlike `relevant` keeps docker/** and frontend/** in # scope — both are built here — and skips only docs-only changes. @@ -236,7 +312,10 @@ jobs: retention-days: 14 report: - needs: [changes, test, e2e] + # `frontend` is in `needs` for the gate below only: it emits no junit, so + # it contributes nothing to the combined report or the baseline. Without it + # a red layout assertion would leave the branch-protection check green. + needs: [changes, test, e2e, frontend] # `always()` so a red tier still posts a report. Any tier may skip on # irrelevant paths; the fail step below distinguishes a legitimate skip # from a real tier failure. @@ -336,7 +415,9 @@ jobs: changes_result="${{ needs.changes.result }}" test_result="${{ needs.test.result }}" e2e_result="${{ needs.e2e.result }}" + frontend_result="${{ needs.frontend.result }}" # A broken filter job skips both tiers, which would read as pass. [ "$changes_result" = "success" ] || exit 1 [ "$test_result" = "success" ] || [ "$test_result" = "skipped" ] || exit 1 [ "$e2e_result" = "success" ] || [ "$e2e_result" = "skipped" ] || exit 1 + [ "$frontend_result" = "success" ] || [ "$frontend_result" = "skipped" ] || exit 1 diff --git a/.gitignore b/.gitignore index 427afc934d..a1435a2855 100644 --- a/.gitignore +++ b/.gitignore @@ -717,3 +717,6 @@ AGENTS.md # Unstract test rig reports/ .test-selection + +# Vitest browser-mode failure screenshots (written on a red layout assertion) +frontend/src/**/__screenshots__/ diff --git a/backend/utils/tests/test_user_context.py b/backend/utils/tests/test_user_context.py new file mode 100644 index 0000000000..09d2c82d0a --- /dev/null +++ b/backend/utils/tests/test_user_context.py @@ -0,0 +1,41 @@ +"""Regression tests for UserContext.get_organization. + +Pins the no-org short-circuit: the lookup must return None without touching the +DB, so it stays evaluable on a DB-less/unmigrated setup. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from utils.user_context import UserContext + + +class TestGetOrganizationNoContext: + def test_returns_none_without_hitting_db(self): + with ( + patch("utils.user_context.StateStore.get", return_value=None), + patch("utils.user_context.Organization.objects.get") as mock_get, + ): + assert UserContext.get_organization() is None + mock_get.assert_not_called() + + def test_empty_identifier_short_circuits(self): + with ( + patch("utils.user_context.StateStore.get", return_value=""), + patch("utils.user_context.Organization.objects.get") as mock_get, + ): + assert UserContext.get_organization() is None + mock_get.assert_not_called() + + def test_identifier_present_looks_up_organization(self): + """Pins the complement, so inverting the guard can't pass unnoticed.""" + sentinel = object() + with ( + patch("utils.user_context.StateStore.get", return_value="org-123"), + patch( + "utils.user_context.Organization.objects.get", return_value=sentinel + ) as mock_get, + ): + assert UserContext.get_organization() is sentinel + mock_get.assert_called_once_with(organization_id="org-123") diff --git a/backend/utils/user_context.py b/backend/utils/user_context.py index 71d63806b5..e0f2d4fc12 100644 --- a/backend/utils/user_context.py +++ b/backend/utils/user_context.py @@ -18,6 +18,9 @@ def set_organization_identifier(organization_identifier: str) -> None: @staticmethod def get_organization() -> Organization | None: organization_id = StateStore.get(Account.ORGANIZATION_ID) + # Skip the query so this stays evaluable on a DB-less/unmigrated setup. + if not organization_id: + return None try: organization: Organization = Organization.objects.get( organization_id=organization_id diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index aa36cbf961..e85b9bf22c 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -39,7 +39,6 @@ services: - ./workflow_data:/data - ${TOOL_REGISTRY_CONFIG_SRC_PATH}:/data/tool_registry_config environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-backend labels: - traefik.enable=true @@ -60,7 +59,6 @@ services: - rabbitmq - db environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-metrics labels: @@ -83,7 +81,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-ide-callback - WORKER_TYPE=ide_callback - WORKER_NAME=ide-callback-worker @@ -107,7 +104,6 @@ services: ports: - "5555:5555" environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-celery-flower volumes: - unstract_data:/data @@ -129,7 +125,6 @@ services: - db - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-celery-beat # Frontend React app @@ -143,7 +138,6 @@ services: - backend - reverse-proxy environment: - - ENVIRONMENT=development # Running platform version, surfaced on the profile page. Reuses the same # ${VERSION} that tags the image, so it always matches what's deployed # (and updates on RC->stable promotion, which only retags — doesn't rebuild). @@ -219,7 +213,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-api-deployment-v2 - WORKER_TYPE=api_deployment - CELERY_QUEUES_API_DEPLOYMENT=${CELERY_QUEUES_API_DEPLOYMENT:-celery_api_deployments} @@ -252,7 +245,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-callback-v2 - WORKER_TYPE=callback - WORKER_NAME=callback-worker-v2 @@ -294,7 +286,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-file-processing-v2 - WORKER_TYPE=file_processing - WORKER_MODE=oss @@ -332,7 +323,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-general-v2 - WORKER_TYPE=general - WORKER_NAME=general-worker-v2 @@ -360,7 +350,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-notification-v2 - WORKER_TYPE=notification - WORKER_NAME=notification-worker-v2 @@ -409,7 +398,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-log-consumer-v2 - WORKER_TYPE=log_consumer - WORKER_NAME=log-consumer-worker-v2 @@ -458,7 +446,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-log-history-scheduler-v2 # Scheduler interval in seconds - LOG_HISTORY_CONSUMER_INTERVAL=${LOG_HISTORY_CONSUMER_INTERVAL:-5} @@ -483,7 +470,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-scheduler-v2 - WORKER_TYPE=scheduler - WORKER_NAME=scheduler-worker-v2 @@ -529,7 +515,6 @@ services: - rabbitmq - platform-service environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-executor-v2 - WORKER_TYPE=executor - WORKER_NAME=executor-worker-v2 diff --git a/frontend/bun.lock b/frontend/bun.lock index 53b71ad79c..8a4b86951d 100644 --- a/frontend/bun.lock +++ b/frontend/bun.lock @@ -62,9 +62,11 @@ "devDependencies": { "@biomejs/biome": "^2.3.13", "@vitejs/plugin-react": "^4.4.0", + "@vitest/browser": "3.2.6", "baseline-browser-mapping": "^2.9.19", "happy-dom": "^20.10.2", "jsdom": "^27.0.1", + "playwright": "^1.61.1", "vite": "^7.3.5", "vite-plugin-svgr": "^4.5.0", "vitest": "^3.2.6", @@ -297,6 +299,8 @@ "@opentelemetry/semantic-conventions": ["@opentelemetry/semantic-conventions@1.39.0", "", {}, "sha512-R5R9tb2AXs2IRLNKLBJDynhkfmx7mX0vi8NkhZb3gUkPWHn6HXk5J8iQ/dql0U3ApfWym4kXXmBDRGO+oeOfjg=="], + "@polka/url": ["@polka/url@1.0.0-next.29", "", {}, "sha512-wwQAWhWSuHaag8c4q/KN/vCoeOJYshAIvMQwD4GpSb3OiZklFfvAgmj0VCBBImRpuF/aFgIRzllXlVX93Jevww=="], + "@posthog/core": ["@posthog/core@1.15.0", "", { "dependencies": { "cross-spawn": "^7.0.6" } }, "sha512-n2/Yy0+qc8xhmlcOFiYqTcGHBZuuaQjVolfFXk7yTCynzdMe8Fx1zYvPPUrbdQK5tWwXyilkzybpqhK6I7aV4Q=="], "@posthog/types": ["@posthog/types@1.336.1", "", {}, "sha512-KSGst/a/HK7GhfLSbwAy35HtU3KjDqjLtq3+PoDlGfbz9SbO0owjc6jo6hAHnMz67QTSvrn/r0xgimDO4NQ+rA=="], @@ -489,7 +493,7 @@ "@svgr/plugin-jsx": ["@svgr/plugin-jsx@8.1.0", "", { "dependencies": { "@babel/core": "^7.21.3", "@svgr/babel-preset": "8.1.0", "@svgr/hast-util-to-babel-ast": "8.0.0", "svg-parser": "^2.0.4" }, "peerDependencies": { "@svgr/core": "*" } }, "sha512-0xiIyBsLlr8quN+WyuxooNW9RJ0Dpr8uOnH/xrCVO8GLUcwHISwj1AG0k+LFzteTkAA0GbX0kj9q6Dk70PTiPA=="], - "@testing-library/dom": ["@testing-library/dom@8.20.1", "", { "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", "@types/aria-query": "^5.0.1", "aria-query": "5.1.3", "chalk": "^4.1.0", "dom-accessibility-api": "^0.5.9", "lz-string": "^1.5.0", "pretty-format": "^27.0.2" } }, "sha512-/DiOQ5xBxgdYRC8LNk7U+RWat0S3qRLeIw3ZIkMQ9kkVlRmwD/Eg8k8CqIpD6GW7u20JIUOfMKbxtiLutpjQ4g=="], + "@testing-library/dom": ["@testing-library/dom@10.4.1", "", { "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", "@types/aria-query": "^5.0.1", "aria-query": "5.3.0", "dom-accessibility-api": "^0.5.9", "lz-string": "^1.5.0", "picocolors": "1.1.1", "pretty-format": "^27.0.2" } }, "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg=="], "@testing-library/jest-dom": ["@testing-library/jest-dom@5.17.0", "", { "dependencies": { "@adobe/css-tools": "^4.0.1", "@babel/runtime": "^7.9.2", "@types/testing-library__jest-dom": "^5.9.1", "aria-query": "^5.0.0", "chalk": "^3.0.0", "css.escape": "^1.5.1", "dom-accessibility-api": "^0.5.6", "lodash": "^4.17.15", "redent": "^3.0.0" } }, "sha512-ynmNeT7asXyH3aSVv4vvX4Rb+0qjOhdNHnO/3vuZNqPmhDpV/+rCSGwQ7bLcmU2cJ4dvoheIO85LQj0IbJHEtg=="], @@ -581,6 +585,8 @@ "@vitejs/plugin-react": ["@vitejs/plugin-react@4.7.0", "", { "dependencies": { "@babel/core": "^7.28.0", "@babel/plugin-transform-react-jsx-self": "^7.27.1", "@babel/plugin-transform-react-jsx-source": "^7.27.1", "@rolldown/pluginutils": "1.0.0-beta.27", "@types/babel__core": "^7.20.5", "react-refresh": "^0.17.0" }, "peerDependencies": { "vite": "^4.2.0 || ^5.0.0 || ^6.0.0 || ^7.0.0" } }, "sha512-gUu9hwfWvvEDBBmgtAowQCojwZmJ5mcLn3aufeCsitijs3+f2NsrPtlAWIR6OPiqljl96GVCUbLe0HyqIpVaoA=="], + "@vitest/browser": ["@vitest/browser@3.2.6", "", { "dependencies": { "@testing-library/dom": "^10.4.0", "@testing-library/user-event": "^14.6.1", "@vitest/mocker": "3.2.6", "@vitest/utils": "3.2.6", "magic-string": "^0.30.17", "sirv": "^3.0.1", "tinyrainbow": "^2.0.0", "ws": "^8.18.2" }, "peerDependencies": { "playwright": "*", "vitest": "3.2.6", "webdriverio": "^7.0.0 || ^8.0.0 || ^9.0.0" }, "optionalPeers": ["playwright", "webdriverio"] }, "sha512-CNjSynGBtAVOMTfQITv6Bc8da4/XTU1izorocbDStjUsynXcgx2FHVssh+10a8bKd/BxoqDdQtuSbYHfk302Wg=="], + "@vitest/expect": ["@vitest/expect@3.2.6", "", { "dependencies": { "@types/chai": "^5.2.2", "@vitest/spy": "3.2.6", "@vitest/utils": "3.2.6", "chai": "^5.2.0", "tinyrainbow": "^2.0.0" } }, "sha512-1+7q9BtaKzEmO+fmNT3kYvoNn5Y71XWAx2Q5HRim4tTVRQVRv4uJFAQ5FbK0OPUeNP/WmVCpxYxoJdvuHVjzBQ=="], "@vitest/mocker": ["@vitest/mocker@3.2.6", "", { "dependencies": { "@vitest/spy": "3.2.6", "estree-walker": "^3.0.3", "magic-string": "^0.30.17" }, "peerDependencies": { "msw": "^2.4.9", "vite": "^5.0.0 || ^6.0.0 || ^7.0.0-0" }, "optionalPeers": ["msw", "vite"] }, "sha512-EZOrpDbkKotFAP7wPAQV1UIyoGOk4oX7ynWhBhLB7v+meMHbQhU16oPpIYGTTe4oFlhpryGpgpcZP/sin3hYuw=="], @@ -833,7 +839,7 @@ "escape-string-regexp": ["escape-string-regexp@4.0.0", "", {}, "sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA=="], - "estree-walker": ["estree-walker@2.0.2", "", {}, "sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w=="], + "estree-walker": ["estree-walker@3.0.3", "", { "dependencies": { "@types/estree": "^1.0.0" } }, "sha512-7RUKfXgSMMkzt6ZuXmqapOurLGPPfgj6l9uRZ7lRGolvk0y2yocc35LdcxKC5PQZdn2DMqioAQ2NoWcrTKmm6g=="], "eventemitter3": ["eventemitter3@5.0.4", "", {}, "sha512-mlsTRyGaPBjPedk6Bvw+aqbsXDtoAyAzm5MO7JgU+yVRyMQ5O8bD4Kcci7BS85f93veegeCPkL8R4GLClnjLFw=="], @@ -875,7 +881,7 @@ "fs.realpath": ["fs.realpath@1.0.0", "", {}, "sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw=="], - "fsevents": ["fsevents@2.3.3", "", { "os": "darwin" }, "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw=="], + "fsevents": ["fsevents@2.3.2", "", { "os": "darwin" }, "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA=="], "function-bind": ["function-bind@1.1.2", "", {}, "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA=="], @@ -1211,6 +1217,8 @@ "mri": ["mri@1.2.0", "", {}, "sha512-tzzskb3bG8LvYGFF/mDTpq3jpI6Q9wc3LEmBaghu+DdCssd1FakN7Bc0hVNmEyGq1bq3RgfkCb3cmQLpNPOroA=="], + "mrmime": ["mrmime@2.0.1", "", {}, "sha512-Y3wQdFg2Va6etvQ5I82yUhGdsKrcYox6p7FfL1LbK2J4V01F9TGlepTIhnK24t7koZibmg82KGglhA1XK5IsLQ=="], + "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], "nan": ["nan@2.25.0", "", {}, "sha512-0M90Ag7Xn5KMLLZ7zliPWP3rT90P6PN+IzVFS0VqmnPktBk3700xUVv8Ikm9EUaUE5SDWdp/BIxdENzVznpm1g=="], @@ -1271,6 +1279,10 @@ "picomatch": ["picomatch@4.0.3", "", {}, "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q=="], + "playwright": ["playwright@1.61.1", "", { "dependencies": { "playwright-core": "1.61.1" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-DWnY5o3YbLWK4GovuAVwpqL+1VwGNdUGrRr++8j8PtQQzvAVZUIMjKQ90fY689sEJZJBbZVw1rXaOKSTitkzPQ=="], + + "playwright-core": ["playwright-core@1.61.1", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-h7Qlt6m4REp25qvIdvbDtVmD4LqVXfpRxhORv9L0jzETM05p4fuPJ3dKyuSXQxDSbXnmS79HAgi9589lGSpLkg=="], + "possible-typed-array-names": ["possible-typed-array-names@1.1.0", "", {}, "sha512-/+5VFTchJDoVj3bhoqi6UeymcD00DAwb1nJwamzPvHEszJ4FpF6SNNbUbOS8yI56qHzdV8eK0qEfOSiodkTdxg=="], "postcss": ["postcss@8.5.6", "", { "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" } }, "sha512-3Ybi1tAuwAP9s0r1UQ2J4n5Y0G05bJkpUIO0/bI9MhwmD70S5aTWbXGBwxHrelT+XM1k6dM0pk+SwNkpTRN7Pg=="], @@ -1487,6 +1499,8 @@ "simple-get": ["simple-get@3.1.1", "", { "dependencies": { "decompress-response": "^4.2.0", "once": "^1.3.1", "simple-concat": "^1.0.0" } }, "sha512-CQ5LTKGfCpvE1K0n2us+kuMPbk/q0EKl82s4aheV9oXjFEz6W/Y7oQFVJuU6QG77hRT4Ghb5RURteF5vnWjupA=="], + "sirv": ["sirv@3.0.2", "", { "dependencies": { "@polka/url": "^1.0.0-next.24", "mrmime": "^2.0.0", "totalist": "^3.0.0" } }, "sha512-2wcC/oGxHis/BoHkkPwldgiPSYcpZK3JU28WoMVv55yHJgcZ8rlXvuG9iZggz+sU1d4bRgIGASwyWqjxu3FM0g=="], + "slash": ["slash@3.0.0", "", {}, "sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q=="], "snake-case": ["snake-case@3.0.4", "", { "dependencies": { "dot-case": "^3.0.4", "tslib": "^2.0.3" } }, "sha512-LAOh4z89bGQvl9pFfNF8V146i7o7/CqFPbqzYgP+yYzDIDeS9HaNFtXABamRW+AQzEVODcvE79ljJ+8a9YSdMg=="], @@ -1565,6 +1579,8 @@ "toggle-selection": ["toggle-selection@1.0.6", "", {}, "sha512-BiZS+C1OS8g/q2RRbJmy59xpyghNBqrr6k5L/uKBGRsTfxmu3ffiRnd8mlGPUVayg8pvfi5urfnu8TU7DVOkLQ=="], + "totalist": ["totalist@3.0.1", "", {}, "sha512-sf4i37nQ2LBx4m3wB74y+ubopq6W/dIzXg0FDGjsYnZHVa1Da8FH853wlL2gtUhg+xJXjfk3kUZS3BRoQeoQBQ=="], + "tough-cookie": ["tough-cookie@6.0.1", "", { "dependencies": { "tldts": "^7.0.5" } }, "sha512-LktZQb3IeoUWB9lqR5EWTHgW/VTITCXg4D21M+lvybRVdylLrRMnqaIONLVb5mav8vM19m44HIcGq4qASeu2Qw=="], "tr46": ["tr46@6.0.0", "", { "dependencies": { "punycode": "^2.3.1" } }, "sha512-bLVMLPtstlZ4iMQHpFHTR7GAGj2jxi8Dg0s2h2MafAE4uSWF98FC/3MomU51iQAMf8/qDUbKWf5GxuvvVcXEhw=="], @@ -1711,15 +1727,17 @@ "@rjsf/antd/rc-picker": ["rc-picker@2.7.6", "", { "dependencies": { "@babel/runtime": "^7.10.1", "classnames": "^2.2.1", "date-fns": "2.x", "dayjs": "1.x", "moment": "^2.24.0", "rc-trigger": "^5.0.4", "rc-util": "^5.37.0", "shallowequal": "^1.1.0" }, "peerDependencies": { "react": ">=16.9.0", "react-dom": ">=16.9.0" } }, "sha512-H9if/BUJUZBOhPfWcPeT15JUI3/ntrG9muzERrXDkSoWmDj4yzmBvumozpxYrHwjcKnjyDGAke68d+whWwvhHA=="], + "@rollup/pluginutils/estree-walker": ["estree-walker@2.0.2", "", {}, "sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w=="], + "@svgr/hast-util-to-babel-ast/entities": ["entities@4.5.0", "", {}, "sha512-V0hjH4dGPh9Ao5p0MoRY6BVqtwCjhz6vI5LT8AJ55H+4g9/4vbHx1I54fS0XuclLhDHArPQCiMjDxjaL8fPxhw=="], - "@testing-library/dom/aria-query": ["aria-query@5.1.3", "", { "dependencies": { "deep-equal": "^2.0.5" } }, "sha512-R5iJ5lkuHybztUfuOAznmboyjWq8O6sqNqtK7CLOqdydi54VNbORp49mb14KbWgG1QD3JFO9hJdZ+y4KutfdOQ=="], + "@testing-library/dom/aria-query": ["aria-query@5.3.0", "", { "dependencies": { "dequal": "^2.0.3" } }, "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A=="], - "@testing-library/dom/chalk": ["chalk@4.1.2", "", { "dependencies": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" } }, "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA=="], + "@testing-library/react/@testing-library/dom": ["@testing-library/dom@8.20.1", "", { "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", "@types/aria-query": "^5.0.1", "aria-query": "5.1.3", "chalk": "^4.1.0", "dom-accessibility-api": "^0.5.9", "lz-string": "^1.5.0", "pretty-format": "^27.0.2" } }, "sha512-/DiOQ5xBxgdYRC8LNk7U+RWat0S3qRLeIw3ZIkMQ9kkVlRmwD/Eg8k8CqIpD6GW7u20JIUOfMKbxtiLutpjQ4g=="], "@types/jest/pretty-format": ["pretty-format@30.2.0", "", { "dependencies": { "@jest/schemas": "30.0.5", "ansi-styles": "^5.2.0", "react-is": "^18.3.1" } }, "sha512-9uBdv/B4EefsuAL+pWqueZyZS2Ba+LxfFeQ9DN14HU4bN8bhaxKdkpjpB6fs9+pSjIBu+FXQHImEg8j/Lw0+vA=="], - "@vitest/mocker/estree-walker": ["estree-walker@3.0.3", "", { "dependencies": { "@types/estree": "^1.0.0" } }, "sha512-7RUKfXgSMMkzt6ZuXmqapOurLGPPfgj6l9uRZ7lRGolvk0y2yocc35LdcxKC5PQZdn2DMqioAQ2NoWcrTKmm6g=="], + "@vitest/browser/@testing-library/user-event": ["@testing-library/user-event@14.6.1", "", { "peerDependencies": { "@testing-library/dom": ">=7.21.4" } }, "sha512-vq7fv0rnt+QTXgPxr5Hjc210p6YKq2kmdziLgnsZGgLJ9e6VAShx1pACLuRjd/AS/sr7phAR58OIIpf0LlmQNw=="], "babel-plugin-macros/cosmiconfig": ["cosmiconfig@7.1.0", "", { "dependencies": { "@types/parse-json": "^4.0.0", "import-fresh": "^3.2.1", "parse-json": "^5.0.0", "path-type": "^4.0.0", "yaml": "^1.10.0" } }, "sha512-AdmX6xUzdNASswsFtmwSt7Vj8po9IuqXm0UXz7QKPuEUmPB4XyjGfaAr2PSuELMwkRMVH1EpIkX5bTZGRB3eCA=="], @@ -1785,6 +1803,8 @@ "refractor/prismjs": ["prismjs@1.27.0", "", {}, "sha512-t13BGPUlFDR7wRB5kQDG4jjl7XeuH6jbJGt11JHPL96qwsEHNX2+68tFXqc1/k+/jALsbSWJKUOT/hcYAZ5LkA=="], + "rollup/fsevents": ["fsevents@2.3.3", "", { "os": "darwin" }, "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw=="], + "stack-utils/escape-string-regexp": ["escape-string-regexp@2.0.0", "", {}, "sha512-UpzcLCXolUWcNu5HtVMHYdXJjArjsF9C0aNnquZYY4uW/Vu0miy5YoWvbV345HauVvcAUnpRuhMMcqTcGOY2+w=="], "string-width/emoji-regex": ["emoji-regex@8.0.0", "", {}, "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A=="], @@ -1795,12 +1815,18 @@ "unified/is-plain-obj": ["is-plain-obj@4.1.0", "", {}, "sha512-+Pgi+vMuUNkJyExiMBt5IlFoMyKnr5zhJ4Uspz58WOhBF5QoIZkFyNHIbBAtHwzVAgk5RtndVNsDRN61/mmDqg=="], + "vite/fsevents": ["fsevents@2.3.3", "", { "os": "darwin" }, "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw=="], + "@mapbox/node-pre-gyp/https-proxy-agent/agent-base": ["agent-base@6.0.2", "", { "dependencies": { "debug": "4" } }, "sha512-RZNwNclF7+MS/8bDg70amg32dyeZGZxiDuQmZxKLAlQjr3jGyLx+4Kkk58UO7D2QdgFIQCovuSuZESne6RG6XQ=="], "@react-awesome-query-builder/ui/react-redux/@types/use-sync-external-store": ["@types/use-sync-external-store@0.0.3", "", {}, "sha512-EwmlvuaxPNej9+T4v5AuBPJa2x2UOJVdjCtDHgcDqitUeOtjnJKJ+apYjVcAoBEMjKW1VVFGZLUb5+qqa09XFA=="], "@rjsf/antd/rc-picker/date-fns": ["date-fns@2.30.0", "", { "dependencies": { "@babel/runtime": "^7.21.0" } }, "sha512-fnULvOpxnC5/Vg3NCiWelDsLiUc9bRwAPs/+LfTLNvetFCtCTN+yQz15C/fs4AwX1R9K5GLtLfn8QW+dWisaAw=="], + "@testing-library/react/@testing-library/dom/aria-query": ["aria-query@5.1.3", "", { "dependencies": { "deep-equal": "^2.0.5" } }, "sha512-R5iJ5lkuHybztUfuOAznmboyjWq8O6sqNqtK7CLOqdydi54VNbORp49mb14KbWgG1QD3JFO9hJdZ+y4KutfdOQ=="], + + "@testing-library/react/@testing-library/dom/chalk": ["chalk@4.1.2", "", { "dependencies": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" } }, "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA=="], + "@types/jest/pretty-format/ansi-styles": ["ansi-styles@5.2.0", "", {}, "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA=="], "fs-minipass/minipass/yallist": ["yallist@4.0.0", "", {}, "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A=="], diff --git a/frontend/package.json b/frontend/package.json index ba164307ee..eaaf921e23 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -62,7 +62,10 @@ "start": "vite", "build": "vite build", "preview": "vite preview", - "test": "vitest", + "test": "vitest run", + "test:unit": "vitest run --project unit", + "test:layout": "vitest run --project layout", + "test:watch": "vitest", "lint": "biome lint src/", "lint:fix": "biome lint --write src/", "format": "biome format src/", @@ -87,9 +90,11 @@ "devDependencies": { "@biomejs/biome": "^2.3.13", "@vitejs/plugin-react": "^4.4.0", + "@vitest/browser": "3.2.6", "baseline-browser-mapping": "^2.9.19", "happy-dom": "^20.10.2", "jsdom": "^27.0.1", + "playwright": "^1.61.1", "vite": "^7.3.5", "vite-plugin-svgr": "^4.5.0", "vitest": "^3.2.6" diff --git a/frontend/src/components/widgets/list-view/ListView.css b/frontend/src/components/widgets/list-view/ListView.css index 0e7ea8c8cf..7ea9b43bb6 100644 --- a/frontend/src/components/widgets/list-view/ListView.css +++ b/frontend/src/components/widgets/list-view/ListView.css @@ -63,9 +63,24 @@ .list-view-meta { align-items: center; display: flex; - flex: 0 1 auto; + /* must not shrink: its columns are fixed below, so a shrinking box would + let them spill over the action icons. The left column absorbs the + shortfall instead — it ellipsizes, this doesn't. */ + flex: 0 0 auto; gap: 20px; - min-width: 0; +} + +/* The meta cluster is anchored on its right edge (the left column grows to + absorb the row's slack), so any column that sizes to its content pushes + the ones before it sideways, row by row. Fixed widths keep every row's + columns at the same x. */ +.list-view-meta .adapters-list-profile-container { + flex: 0 0 240px; + justify-content: flex-start; +} + +.list-view-meta .list-view-modified-container { + flex: 0 0 170px; } .list-view-meta .shared-username { @@ -101,6 +116,8 @@ .adapters-list-title { font-size: 16px; + /* match the agentic Prompt Studio project list */ + font-weight: 600; } .adapter-cover-img { diff --git a/frontend/src/components/widgets/list-view/ListView.layout.test.jsx b/frontend/src/components/widgets/list-view/ListView.layout.test.jsx new file mode 100644 index 0000000000..a768134072 --- /dev/null +++ b/frontend/src/components/widgets/list-view/ListView.layout.test.jsx @@ -0,0 +1,249 @@ +import { describe, expect, it } from "vitest"; +import { useSessionStore } from "../../../store/session-store"; +import { + expectAlignedX, + expectContained, + expectNoHorizontalOverflow, + expectNoHorizontalOverlap, + expectTruncated, +} from "../../../test-utils/layout/assertions"; +import { + mountAtWidth, + queryAll, + rowsOf, +} from "../../../test-utils/layout/mount"; +import { ListView } from "./ListView"; + +// The list row is a table drawn with flexbox: four columns that must line up +// across rows even though every row lays itself out independently. These tests +// hold that contract. Two shipped regressions motivated them — an owner column +// that sized itself to its own text and so started at a different x on every +// row, and pinned columns inside a shrinkable wrapper that spilled out of it +// and printed over the action icons. Neither is visible in a CSS diff. + +const SESSION_EMAIL = "me@example.com"; + +// The widths .list-view-wrapper itself declares support for (min 800, max +// 1400) plus the midpoint. Keep in step with ListView.css if that range moves. +const WIDTHS = [800, 1100, 1400]; + +// Owner labels of wildly different lengths are the whole point: a column that +// sizes to content only misaligns when its content differs row to row. +// Long enough that no plausible width cap could ever fit it: the truncation +// case must fail because truncation broke, never because someone widened a +// column. +const LONG_EMAIL = `a-really-long-service-account-name-${"x".repeat(300)}@example.com`; + +const ITEMS = [ + { + id: "1", + tool_name: "S3", + description: "Short one", + created_by_email: SESSION_EMAIL, + created_at: "2026-01-02T03:04:05Z", + modified_at: "2026-07-20T03:04:05Z", + }, + { + id: "2", + tool_name: + "A deliberately long project name that has to ellipsize somewhere", + description: + "A long description that runs past the width cap and gets clipped with a tooltip", + created_by_email: LONG_EMAIL, + created_at: "2025-03-02T03:04:05Z", + modified_at: "2025-04-02T03:04:05Z", + }, + { + // No description: the left column collapses to a single line, so the row + // is shorter than its neighbours. Column alignment must not care. + id: "3", + tool_name: "KYC extractor", + created_by_email: "someone.else@example.com", + created_at: "2026-06-02T03:04:05Z", + modified_at: "2026-07-21T03:04:05Z", + }, + { + // Membership model: renders "Me +2" rather than an address. + id: "4", + tool_name: "Co-owned invoice parser", + description: "Owned with two other people", + co_owners_count: 3, + is_owner: true, + created_at: "2026-07-01T03:04:05Z", + modified_at: "2026-07-22T03:04:05Z", + }, +]; + +// Handlers only need to exist: their presence decides which columns and +// controls ListView renders, and nothing here clicks anything. +const noop = () => undefined; + +// Adapters and Prompt Studio: owner + updated, share and co-owner enabled. +const fullConfig = { + listOfTools: ITEMS, + handleEdit: noop, + handleDelete: noop, + handleShare: noop, + handleCoOwner: noop, + titleProp: "tool_name", + descriptionProp: "description", + idProp: "id", + showOwner: true, + showModified: true, + type: "Tool", +}; + +// Connectors and Workflows: owner only, no updated column, no co-owner button +// (so the badge renders as a div rather than a button). +const ownerOnlyConfig = { + ...fullConfig, + handleShare: undefined, + handleCoOwner: undefined, + showModified: false, + centered: true, +}; + +const mountList = (props, width) => { + useSessionStore.setState({ sessionDetails: { email: SESSION_EMAIL } }); + return mountAtWidth(, width); +}; + +describe("ListView row layout", () => { + it("renders the owner column as a straight column, whatever the owner is called", async () => { + const { wrapper } = await mountList(fullConfig, 1100); + + const owners = queryAll(wrapper, ".adapters-list-profile-container"); + expect(owners).toHaveLength(ITEMS.length); + // Guard the fixture itself: if every row rendered the same label, the + // alignment assertion below would pass for the wrong reason. + const labels = queryAll(wrapper, ".shared-username").map((el) => + el.textContent.trim(), + ); + expect(new Set(labels).size).toBeGreaterThan(1); + + expectAlignedX(owners, "owner column"); + }); + + it("renders the updated column as a straight column, whatever the timestamps are", async () => { + const { wrapper } = await mountList(fullConfig, 1100); + + const updated = queryAll(wrapper, ".list-view-modified-container"); + expect(updated).toHaveLength(ITEMS.length); + const labels = queryAll(wrapper, ".list-view-modified-text").map((el) => + el.textContent.trim(), + ); + expect(new Set(labels).size).toBeGreaterThan(1); + + expectAlignedX(updated, "updated column"); + }); + + it.each( + WIDTHS, + )("keeps the metadata columns inside their box and clear of the actions at %ipx", async (width) => { + const { wrapper } = await mountList(fullConfig, width); + + for (const row of rowsOf(wrapper)) { + const meta = row.querySelector(".list-view-meta"); + const actions = row.querySelector(".action-button-container"); + + for (const column of meta.children) { + expectContained(column, meta, `meta column at ${width}px`); + } + expectNoHorizontalOverlap( + meta, + actions, + `metadata vs row actions at ${width}px`, + ); + // The columns are what a user sees collide, so assert on them too: + // a contained-but-overlapping child would still print over the icons. + for (const column of meta.children) { + expectNoHorizontalOverlap( + column, + actions, + `metadata column vs row actions at ${width}px`, + ); + } + } + }); + + it.each(WIDTHS)("never overflows the row at %ipx", async (width) => { + const { wrapper } = await mountList(fullConfig, width); + + // Rows only: each row is its own clipping context (`overflow: hidden`), so + // asserting on the wrapper could never fail no matter how badly a row + // overflowed. + for (const row of rowsOf(wrapper)) { + expectNoHorizontalOverflow(row, `row at ${width}px`); + } + }); + + it("keeps the columns straight when rows carry icons", async () => { + // The adapter pages render an beside the title, which settles + // after mount; the left column absorbs it, so the meta columns must not + // move. This is the branch those pages actually use. + const { wrapper } = await mountList( + { + ...fullConfig, + iconProp: "icon", + listOfTools: ITEMS.map((item) => ({ + ...item, + icon: "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='38' height='38'%3E%3C/svg%3E", + })), + }, + 1100, + ); + + expectAlignedX( + queryAll(wrapper, ".adapters-list-profile-container"), + "owner column with row icons", + ); + expectAlignedX( + queryAll(wrapper, ".list-view-modified-container"), + "updated column with row icons", + ); + }); + + it("absorbs an over-long owner address by truncating it, not by moving the column", async () => { + const { wrapper } = await mountList(fullConfig, 800); + + const longLabel = queryAll(wrapper, ".shared-username").find((el) => + el.textContent.includes("a-really-long-service-account-name"), + ); + expectTruncated(longLabel, "over-long owner address"); + expectAlignedX( + queryAll(wrapper, ".adapters-list-profile-container"), + "owner column at the narrowest supported width", + ); + }); + + describe("owner-only pages", () => { + it.each( + WIDTHS, + )("stay aligned, contained and overflow-free at %ipx", async (width) => { + const { wrapper } = await mountList(ownerOnlyConfig, width); + + expect(queryAll(wrapper, ".list-view-modified-container")).toHaveLength( + 0, + ); + + expectAlignedX( + queryAll(wrapper, ".adapters-list-profile-container"), + `owner column, owner-only page at ${width}px`, + ); + + for (const row of rowsOf(wrapper)) { + const meta = row.querySelector(".list-view-meta"); + const actions = row.querySelector(".action-button-container"); + for (const column of meta.children) { + expectContained(column, meta, `meta column at ${width}px`); + } + expectNoHorizontalOverlap( + meta, + actions, + `metadata vs row actions at ${width}px`, + ); + expectNoHorizontalOverflow(row, `row at ${width}px`); + } + }); + }); +}); diff --git a/frontend/src/test-utils/layout/assertions.js b/frontend/src/test-utils/layout/assertions.js new file mode 100644 index 0000000000..ec89067f00 --- /dev/null +++ b/frontend/src/test-utils/layout/assertions.js @@ -0,0 +1,75 @@ +import { expect } from "vitest"; + +// Subpixel rounding only. Anything a real layout bug produces is orders of +// magnitude larger (the column drift that motivated these helpers was 181px), +// so a tight epsilon costs nothing and keeps the assertions meaningful. +export const EPSILON = 1.5; + +const box = (el) => el.getBoundingClientRect(); + +const describeAll = (elements, pick) => + elements + .map((el, i) => ` [${i}] ${Math.round(pick(box(el)))}px ${text(el)}`) + .join("\n"); + +const text = (el) => { + const raw = (el.textContent || "").trim().replace(/\s+/g, " "); + return raw.length > 40 ? `${raw.slice(0, 40)}…` : raw; +}; + +/** + * Every element starts at the same x — i.e. they read as a column. + * + * This is the assertion that survives redesigns: it says nothing about where + * the column is, only that it is straight. Padding, fonts and design tokens + * can all change without touching it; only a column that sizes itself to its + * own content breaks it. + */ +export const expectAlignedX = (elements, label) => { + expect(elements.length, `${label}: nothing to align`).toBeGreaterThan(1); + const xs = elements.map((el) => box(el).x); + const spread = Math.max(...xs) - Math.min(...xs); + expect( + spread, + `${label}: expected one straight column, got ${spread.toFixed(1)}px of drift\n${describeAll(elements, (b) => b.x)}`, + ).toBeLessThanOrEqual(EPSILON); +}; + +/** `child` stays inside `parent` horizontally. */ +export const expectContained = (child, parent, label) => { + const c = box(child); + const p = box(parent); + const overflow = Math.max(p.left - c.left, c.right - p.right); + expect( + overflow, + `${label}: child escapes its container by ${overflow.toFixed(1)}px — a rigid child inside a shrinkable box spills instead of shrinking with it\n child ${Math.round(c.left)}→${Math.round(c.right)} ${text(child)}\n parent ${Math.round(p.left)}→${Math.round(p.right)}`, + ).toBeLessThanOrEqual(EPSILON); +}; + +/** `leftEl` ends before `rightEl` begins — they never sit on top of each other. */ +export const expectNoHorizontalOverlap = (leftEl, rightEl, label) => { + const a = box(leftEl); + const b = box(rightEl); + const overlap = a.right - b.left; + expect( + overlap, + `${label}: elements overlap by ${overlap.toFixed(1)}px\n ${Math.round(a.left)}→${Math.round(a.right)} ${text(leftEl)}\n ${Math.round(b.left)}→${Math.round(b.right)} ${text(rightEl)}`, + ).toBeLessThanOrEqual(EPSILON); +}; + +/** Content fits the element instead of forcing it wider. */ +export const expectNoHorizontalOverflow = (el, label) => { + const overflow = el.scrollWidth - el.clientWidth; + expect( + overflow, + `${label}: content is ${overflow}px wider than its box\n ${text(el)}`, + ).toBeLessThanOrEqual(1); +}; + +/** Truncation engaged — the layout absorbed the content, not the reverse. */ +export const expectTruncated = (el, label) => { + expect( + el.scrollWidth, + `${label}: expected the text to be clipped and tooltip-backed, but it fits — the fixture no longer exercises overflow\n ${text(el)}`, + ).toBeGreaterThan(el.clientWidth); +}; diff --git a/frontend/src/test-utils/layout/mount.jsx b/frontend/src/test-utils/layout/mount.jsx new file mode 100644 index 0000000000..014ee52fd9 --- /dev/null +++ b/frontend/src/test-utils/layout/mount.jsx @@ -0,0 +1,81 @@ +import { render } from "@testing-library/react"; +import { MemoryRouter } from "react-router-dom"; +import { expect, onTestFinished } from "vitest"; + +const settle = async () => { + // Fonts change text metrics and antd injects its CSS-in-JS on mount; both + // must be done before anything is measured. + await document.fonts.ready; + await new Promise((resolve) => requestAnimationFrame(resolve)); +}; + +/** + * Mount `ui` with the list rendered at `width` px and wait for layout to settle. + * + * Returns the wrapper element plus the usual RTL result. + */ +export const mountAtWidth = async (ui, width) => { + const host = document.createElement("div"); + host.style.width = `${width}px`; + document.body.appendChild(host); + // Testing Library unmounts the React tree but leaves the container behind, + // and every test in a file shares one browser page — so without this the + // body accumulates a dead host per mount, alongside the portals antd hangs + // off it. + onTestFinished(() => host.remove()); + + const result = render({ui}, { + container: host, + }); + await settle(); + + const wrapper = host.querySelector(".list-view-wrapper"); + expect(wrapper, "list did not render").not.toBeNull(); + + // ListView sizes itself as a percentage of an ancestor that also carries + // padding, so the host width needed for a given list width can't be computed + // up front. Converge on it instead — measure, correct proportionally, repeat. + // Deriving it beats hard-coding the percentage: a design tweak there changes + // nothing this suite asserts and must not turn every case red. + for (let attempt = 0; attempt < 5; attempt += 1) { + const measured = wrapper.getBoundingClientRect().width; + if (Math.abs(measured - width) <= 0.5) { + break; + } + const hostWidth = Number.parseFloat(host.style.width); + host.style.width = `${hostWidth + (width - measured) * (hostWidth / measured)}px`; + await settle(); + } + + // The width is also clamped (min/max on the wrapper). Without this guard a + // clamp would silently collapse three "different" widths into three runs of + // the same one, and the suite would look broader than it is. + const actual = wrapper.getBoundingClientRect().width; + expect( + Math.abs(actual - width), + `asked for a ${width}px list but got ${actual.toFixed(0)}px — the width is being clamped, so this case is not testing what it claims`, + ).toBeLessThanOrEqual(1.5); + + return { ...result, wrapper, host }; +}; + +/** + * The rows of a rendered list. + * + * Asserts there are some: `.ant-list-item` is antd's class, not ours, so a + * major bump could rename it — and every per-row assertion in a suite like + * this one lives inside `for (const row of rowsOf(...))`. Silence there would + * read as green. + */ +export const rowsOf = (wrapper) => { + const rows = [...wrapper.querySelectorAll(".ant-list-item")]; + expect( + rows.length, + "no list rows found — the row selector no longer matches, so every per-row assertion would pass vacuously", + ).toBeGreaterThan(0); + return rows; +}; + +export const queryAll = (wrapper, selector) => [ + ...wrapper.querySelectorAll(selector), +]; diff --git a/frontend/src/test-utils/layout/setup.js b/frontend/src/test-utils/layout/setup.js new file mode 100644 index 0000000000..25bf9e771a --- /dev/null +++ b/frontend/src/test-utils/layout/setup.js @@ -0,0 +1,12 @@ +import "@testing-library/jest-dom"; + +// Layout is measured, so nothing may still be moving when we measure it. +// antd animates list items, tooltips and buttons on mount. +const style = document.createElement("style"); +style.textContent = `*, *::before, *::after { + animation-duration: 0s !important; + animation-delay: 0s !important; + transition-duration: 0s !important; + transition-delay: 0s !important; +}`; +document.head.appendChild(style); diff --git a/frontend/vite-plugins/optional-plugin-imports.js b/frontend/vite-plugins/optional-plugin-imports.js new file mode 100644 index 0000000000..d6c2058b5d --- /dev/null +++ b/frontend/vite-plugins/optional-plugin-imports.js @@ -0,0 +1,83 @@ +import fs from "fs"; +import path from "path"; + +const EMPTY_MODULE_ID = "\0optional-plugin-empty"; +const EMPTY_ASSET_MODULE_ID = "\0optional-plugin-empty-asset"; + +const ASSET_EXTENSIONS = new Set([ + ".svg", + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".ico", + ".bmp", + ".tiff", +]); + +// Rollup plugin that resolves missing optional plugin imports to an empty +// module instead of failing the build. This lets the existing +// `try { await import("./plugins/...") } catch {}` pattern work at build +// time: Rollup will bundle an empty module for any plugin path that does +// not exist on disk, and the catch block handles the rest at runtime. +// +// Asset imports (images, SVGs, etc.) are resolved to a module that exports +// an empty string as default, so static `import logo from "..."` statements +// don't break the build. +export function optionalPluginImports() { + return { + name: "optional-plugin-imports", + resolveId(source, importer) { + if (!importer) { + return null; + } + + // Only handle relative imports + if (!source.startsWith(".")) { + return null; + } + + // Strip query strings and hashes (e.g. "./logo.svg?react" → "./logo.svg") + // so path.extname and fs.existsSync work correctly. + const sourcePath = source.split("?")[0].split("#")[0]; + const resolved = path.resolve(path.dirname(importer), sourcePath); + + // Only handle imports that resolve within a plugins directory. + // This covers both cross-plugin imports (e.g. "../plugins/foo") + // and intra-plugin sibling imports (e.g. "./TrialMessage" from + // within plugins/login-form/). + if (!resolved.includes("/plugins/")) { + return null; + } + + // Check common extensions + const extensions = ["", ".js", ".jsx", ".ts", ".tsx"]; + const exists = extensions.some( + (ext) => + fs.existsSync(resolved + ext) || + fs.existsSync(path.join(resolved, "index" + (ext || ".js"))), + ); + + if (!exists) { + // Asset files need a default export so static imports work. + const ext = path.extname(sourcePath).toLowerCase(); + if (ASSET_EXTENSIONS.has(ext)) { + return EMPTY_ASSET_MODULE_ID; + } + return EMPTY_MODULE_ID; + } + + return null; + }, + load(id) { + if (id === EMPTY_MODULE_ID) { + return "throw new Error('Optional plugin not available');"; + } + if (id === EMPTY_ASSET_MODULE_ID) { + return "export default '';"; + } + return null; + }, + }; +} diff --git a/frontend/vite.config.js b/frontend/vite.config.js index 9ead29294f..14e3dd9310 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -1,83 +1,9 @@ -import react from "@vitejs/plugin-react"; -import fs from "fs"; import path from "path"; +import react from "@vitejs/plugin-react"; import { defineConfig, loadEnv } from "vite"; import svgr from "vite-plugin-svgr"; -const EMPTY_MODULE_ID = "\0optional-plugin-empty"; -const EMPTY_ASSET_MODULE_ID = "\0optional-plugin-empty-asset"; - -const ASSET_EXTENSIONS = new Set([ - ".svg", - ".png", - ".jpg", - ".jpeg", - ".gif", - ".webp", - ".ico", - ".bmp", - ".tiff", -]); - -// Rollup plugin that resolves missing optional plugin imports to an empty -// module instead of failing the build. This lets the existing -// `try { await import("./plugins/...") } catch {}` pattern work at build -// time: Rollup will bundle an empty module for any plugin path that does -// not exist on disk, and the catch block handles the rest at runtime. -// -// Asset imports (images, SVGs, etc.) are resolved to a module that exports -// an empty string as default, so static `import logo from "..."` statements -// don't break the build. -function optionalPluginImports() { - return { - name: "optional-plugin-imports", - resolveId(source, importer) { - if (!importer) return null; - - // Only handle relative imports - if (!source.startsWith(".")) return null; - - // Strip query strings and hashes (e.g. "./logo.svg?react" → "./logo.svg") - // so path.extname and fs.existsSync work correctly. - const sourcePath = source.split("?")[0].split("#")[0]; - const resolved = path.resolve(path.dirname(importer), sourcePath); - - // Only handle imports that resolve within a plugins directory. - // This covers both cross-plugin imports (e.g. "../plugins/foo") - // and intra-plugin sibling imports (e.g. "./TrialMessage" from - // within plugins/login-form/). - if (!resolved.includes("/plugins/")) return null; - - // Check common extensions - const extensions = ["", ".js", ".jsx", ".ts", ".tsx"]; - const exists = extensions.some( - (ext) => - fs.existsSync(resolved + ext) || - fs.existsSync(path.join(resolved, "index" + (ext || ".js"))), - ); - - if (!exists) { - // Asset files need a default export so static imports work. - const ext = path.extname(sourcePath).toLowerCase(); - if (ASSET_EXTENSIONS.has(ext)) { - return EMPTY_ASSET_MODULE_ID; - } - return EMPTY_MODULE_ID; - } - - return null; - }, - load(id) { - if (id === EMPTY_MODULE_ID) { - return "throw new Error('Optional plugin not available');"; - } - if (id === EMPTY_ASSET_MODULE_ID) { - return "export default '';"; - } - return null; - }, - }; -} +import { optionalPluginImports } from "./vite-plugins/optional-plugin-imports"; // https://vite.dev/config/ export default defineConfig(({ mode }) => { diff --git a/frontend/vitest.config.mjs b/frontend/vitest.config.mjs index b315886669..d2aca78e0b 100644 --- a/frontend/vitest.config.mjs +++ b/frontend/vitest.config.mjs @@ -1,8 +1,23 @@ import react from "@vitejs/plugin-react"; import { defineConfig } from "vitest/config"; +import { optionalPluginImports } from "./vite-plugins/optional-plugin-imports"; + +// Two projects, because they need different environments: +// unit — happy-dom, fast, but no layout engine (every rect measures 0) +// layout — real Chromium via playwright, the only place geometry is real +// `bun run test` runs both; `test:unit` / `test:layout` run one. export default defineConfig({ plugins: [ + // src/plugins/** is cloud content: gitignored, absent from OSS checkouts + // and from CI. Vite resolves those imports at transform time and fails the + // module outright, so tests need the same treatment the production build + // gives them — the very same plugin, not a lookalike. It makes a missing + // plugin *throw* on import, which is the signal the runtime fallbacks in + // pluginRegistry and friends match on; stubbing it to an empty module + // instead would make every one of those fallbacks silently take the wrong + // branch under test. + optionalPluginImports(), react({ include: "**/*.{jsx,js}", }), @@ -12,8 +27,43 @@ export default defineConfig({ include: /src\/.*\.jsx?$/, }, test: { - globals: true, - environment: "happy-dom", - setupFiles: "./src/setupTests.js", + projects: [ + { + extends: true, + test: { + name: "unit", + globals: true, + environment: "happy-dom", + setupFiles: "./src/setupTests.js", + include: ["src/**/*.test.{js,jsx}"], + // Layout specs need a browser; they are the other project. + exclude: ["src/**/*.layout.test.{js,jsx}"], + }, + }, + { + extends: true, + test: { + name: "layout", + globals: true, + include: ["src/**/*.layout.test.{js,jsx}"], + setupFiles: "./src/test-utils/layout/setup.js", + browser: { + enabled: true, + provider: "playwright", + headless: true, + screenshotFailures: true, + instances: [ + { + browser: "chromium", + // Fixed so a runner's default window size can never change + // what is measured. Cases size the list themselves; this only + // has to be wider than the widest case. + viewport: { width: 1600, height: 900 }, + }, + ], + }, + }, + }, + ], }, }); diff --git a/tests/README.md b/tests/README.md index 982b12f553..3628cd82c1 100644 --- a/tests/README.md +++ b/tests/README.md @@ -158,7 +158,7 @@ The `platform` pytest fixture in `tests/e2e/conftest.py` reads those env vars; e Execute-path e2e tests must not call a real provider, so the rig sets `UNSTRACT_LLM_MOCK_RESPONSE` (default `MOCK_LLM_OK`) before boot, for any runtime and treating an exported empty string as unset. The test overlay forwards it into the workers, and `unstract.sdk1.llm` passes it to litellm as `mock_response`: for the non-streaming completion path litellm returns the string verbatim with fixed usage (10 prompt / 20 completion / 30 total), so both the answer and the token counts are exact-assertable. Streaming (`stream_complete`) goes through a different litellm path whose usage differs, so don't assume 10/20/30 there. Sentinels like `litellm.RateLimitError` force error paths. Unset (the production default) the hook is a no-op. -Mocking needs two conditions, not one: `ENVIRONMENT` must also be `test` or `development`. Production sets neither variable, so a stray mock var alone cannot fake completions and their billing — and a refusal is logged rather than silent, since setting the var at all means someone expected mocking. `development` is allowed because that is what base compose sets on the workers that run the injection; the test overlay pins `ENVIRONMENT=test` on those same two workers explicitly, so the tier can't lose its mock to a base-compose edit. +The variable being set is the only condition, deliberately. A second gate on `ENVIRONMENT` was tried and removed: it did not defend against the case it was written for — a worker env block copied out of this overlay carries both variables together — and compose declared `ENVIRONMENT` on every service anyway, so any deployment derived from it satisfied the gate. Nothing read that variable, so it was dropped everywhere along with the gate. What remains is that the hatch is off unless someone sets it, and that it warns once per process while active. Making fake spend safe to *detect* rather than trying to prevent the config wants the usage record itself to say it was mocked. A CI/dev override wins (the rig only fills an unset value). Running these tests under the rig **fails** if the var is missing; running **without** the rig just skips the execute-path tests — export it on both sides (your shell and the workers) if you boot the stack yourself. diff --git a/tests/compose/docker-compose.test.yaml b/tests/compose/docker-compose.test.yaml index feb46d6eda..8b18a3ca4b 100644 --- a/tests/compose/docker-compose.test.yaml +++ b/tests/compose/docker-compose.test.yaml @@ -7,33 +7,23 @@ services: # contributor run without setting it. image: unstract/backend:${UNSTRACT_TEST_VERSION:-latest} environment: - - ENVIRONMENT=test # Workers ask the backend for this, so this is the value that normally # takes effect for fan-out. - MAX_PARALLEL_FILE_BATCHES=${MAX_PARALLEL_FILE_BATCHES:-3} platform-service: image: unstract/platform-service:${UNSTRACT_TEST_VERSION:-latest} - environment: - - ENVIRONMENT=test runner: image: unstract/runner:${UNSTRACT_TEST_VERSION:-latest} - environment: - - ENVIRONMENT=test - # Execute-path e2e must never reach a real provider. The delay gives each - # mocked completion a known cost so per-file durations stay comparable, and - # ENVIRONMENT is the mock's second condition — set here rather than inherited - # so the tier can't lose its mock to a base-compose edit. + # Execute-path e2e must never reach a real provider. worker-executor-v2: environment: - - ENVIRONMENT=test - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} worker-file-processing-v2: environment: - - ENVIRONMENT=test - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} # Only the worker's fallback if it can't reach the backend; kept in step so it diff --git a/tests/rig/groups.py b/tests/rig/groups.py index 07b70283ce..3a2c7b25d4 100644 --- a/tests/rig/groups.py +++ b/tests/rig/groups.py @@ -9,6 +9,7 @@ from __future__ import annotations import graphlib +import os from collections.abc import Mapping from dataclasses import dataclass, field from pathlib import Path @@ -26,6 +27,10 @@ REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_MANIFEST = REPO_ROOT / "tests" / "groups.yaml" +# os.pathsep-separated overlay manifests, so a downstream repo can contribute +# groups without editing the base manifest. +EXTRA_MANIFESTS_ENV = "UNSTRACT_RIG_EXTRA_MANIFESTS" + @dataclass(frozen=True) class GroupDefinition: @@ -118,15 +123,20 @@ def expand(self, selected: list[str]) -> list[str]: def load_groups(path: Path | None = None) -> GroupManifest: """Parse the YAML manifest and validate it.""" manifest_path = path or DEFAULT_MANIFEST - raw = yaml.safe_load(manifest_path.read_text()) - if not isinstance(raw, dict) or "groups" not in raw: - raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") + raw = _load_manifest_dict(manifest_path) defaults = raw.get("defaults") or {} groups: dict[str, GroupDefinition] = {} for name, spec in (raw["groups"] or {}).items(): groups[name] = _build_group(name, spec, defaults) + # Merge before validation so cross-manifest `depends_on` and the platform + # gate are checked over the union. Keyed on `path` being omitted rather than + # on its value: an explicit path means "load exactly this", however spelled. + if path is None: + for extra in _extra_manifest_paths(): + defaults = _merge_manifest(groups, extra, defaults) + _validate_no_cycles(groups) _validate_dep_targets_exist(groups) _validate_paths(groups) @@ -135,6 +145,58 @@ def load_groups(path: Path | None = None) -> GroupManifest: return GroupManifest(groups=groups) +def _extra_manifest_paths() -> list[Path]: + """Overlay manifest paths from ``UNSTRACT_RIG_EXTRA_MANIFESTS``. + + Relative paths resolve against ``REPO_ROOT`` so a downstream repo can point + at a manifest it copied into this tree (e.g. ``tests/groups.cloud.yaml``). + """ + raw = os.environ.get(EXTRA_MANIFESTS_ENV, "").strip() + if not raw: + return [] + paths: list[Path] = [] + for entry in filter(None, (e.strip() for e in raw.split(os.pathsep))): + p = Path(entry) + p = p if p.is_absolute() else REPO_ROOT / p + # Name the env var — a bare FileNotFoundError won't say where the path + # came from. + if not p.is_file(): + raise ValueError( + f"{EXTRA_MANIFESTS_ENV}: {entry!r} is not a file (resolved to {p})" + ) + paths.append(p) + return paths + + +def _load_manifest_dict(manifest_path: Path) -> dict[str, Any]: + """Parse a manifest YAML, rejecting anything that is not a ``groups:`` mapping.""" + if not manifest_path.is_file(): + raise ValueError(f"{manifest_path}: manifest file not found") + raw = yaml.safe_load(manifest_path.read_text()) + if not isinstance(raw, dict) or "groups" not in raw: + raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") + return raw + + +def _merge_manifest( + groups: dict[str, GroupDefinition], manifest_path: Path, base_defaults: dict[str, Any] +) -> dict[str, Any]: + """Merge an overlay manifest into ``groups`` in place and return the merged + defaults, so an overlay can also rename the platform gate. Overlay groups + inherit the base ``defaults`` unless they declare their own; a name collision + is an error rather than a silent override. + """ + raw = _load_manifest_dict(manifest_path) + defaults = {**base_defaults, **(raw.get("defaults") or {})} + for name, spec in (raw["groups"] or {}).items(): + if name in groups: + raise ValueError( + f"{manifest_path}: group {name!r} already defined in a prior manifest" + ) + groups[name] = _build_group(name, spec, defaults) + return defaults + + def _build_group( name: str, spec: dict[str, Any], defaults: dict[str, Any] ) -> GroupDefinition: diff --git a/tests/rig/selection.py b/tests/rig/selection.py index a1daac001a..518330767d 100644 --- a/tests/rig/selection.py +++ b/tests/rig/selection.py @@ -7,6 +7,9 @@ The literal ``all`` expands to every group in the manifest. When the result is empty, callers should treat that as "do nothing" rather than silently running everything — the CLI surfaces a clear error. + +With ``--tier``, dep expansion stays inside that tier: tiers run as separate CI +legs, so a cross-tier ``depends_on`` would re-run the same group in every leg. """ from __future__ import annotations @@ -66,7 +69,16 @@ def resolve( if changed_only: selected.update(_groups_for_changed_paths(manifest, base=changed_base)) - return manifest.expand(sorted(selected)) if selected else [] + if not selected: + return [] + + requested = sorted(selected) + expanded = manifest.expand(requested) + if tier is not None: + # Only dep-expanded groups are dropped; an explicit request survives. + keep = set(requested) + expanded = [n for n in expanded if n in keep or manifest.get(n).tier == tier] + return expanded def _groups_for_changed_paths(manifest: GroupManifest, *, base: str) -> set[str]: diff --git a/tests/rig/tests/test_groups.py b/tests/rig/tests/test_groups.py index eabd5b7f05..a8e55a26c2 100644 --- a/tests/rig/tests/test_groups.py +++ b/tests/rig/tests/test_groups.py @@ -232,3 +232,154 @@ def test_group_env_is_frozen() -> None: assert group.env["A"] == "1" with pytest.raises(TypeError): group.env["B"] = "2" # type: ignore[index] + + +def _base_manifest(tmp_path: Path) -> Path: + return _write_manifest( + tmp_path, + """ + version: 1 + groups: + unit-base: + tier: unit + paths: [x] + optional: true + """, + ) + + +def _overlay_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, body: str +) -> Path: + """Point the default manifest at a fixture and register an overlay. + + Overlays only apply when `load_groups` is called with no path, so tests must + drive that call. + """ + base = _base_manifest(tmp_path) + overlay = tmp_path / "groups.cloud.yaml" + overlay.write_text(body) + monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) + return base + + +def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + groups: + unit-cloud: + tier: unit + paths: [y] + depends_on: [unit-base] + optional: true + """, + ) + manifest = load_groups() + assert "unit-cloud" in manifest.names() + # Cross-manifest depends_on resolves against the merged set. + assert "unit-base" in manifest.expand(["unit-cloud"]) + + +def test_extra_manifest_name_collision_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + groups: + unit-base: + tier: unit + paths: [y] + optional: true + """, + ) + with pytest.raises(ValueError, match="already defined"): + load_groups() + + +def test_explicit_manifest_ignores_overlays( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An ad-hoc manifest must not absorb a downstream repo's overlay.""" + base = _base_manifest(tmp_path) + overlay = tmp_path / "groups.cloud.yaml" + overlay.write_text( + """ + version: 1 + groups: + unit-cloud: + tier: unit + paths: [y] + optional: true + """ + ) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) + # `base` is deliberately not the default manifest here. + assert "unit-cloud" not in load_groups(base).names() + + +def test_explicit_default_path_ignores_overlays( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Isolation keys on `path` being omitted, not on which file it names.""" + base = _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + groups: + unit-cloud: + tier: unit + paths: [y] + optional: true + """, + ) + assert "unit-cloud" not in load_groups(base).names() + + +def test_extra_manifest_defaults_applied( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An overlay's own `defaults` reach its groups instead of being dropped.""" + _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + defaults: + parallel: false + timeout_seconds: 42 + groups: + unit-cloud: + tier: unit + paths: [y] + optional: true + """, + ) + group = load_groups().get("unit-cloud") + assert group.parallel is False + assert group.timeout_seconds == 42 + + +def test_extra_manifest_malformed_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _overlay_env(tmp_path, monkeypatch, "- not: a-mapping\n") + with pytest.raises(ValueError, match="expected top-level `groups:` mapping"): + load_groups() + + +def test_extra_manifest_missing_path_names_env_var( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + base = _base_manifest(tmp_path) + monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(tmp_path / "nope.yaml")) + with pytest.raises(ValueError, match="UNSTRACT_RIG_EXTRA_MANIFESTS"): + load_groups() diff --git a/tests/rig/tests/test_selection.py b/tests/rig/tests/test_selection.py index d0ee7ef410..4007c1963c 100644 --- a/tests/rig/tests/test_selection.py +++ b/tests/rig/tests/test_selection.py @@ -76,3 +76,50 @@ def test_tier_only_selects_that_tier_and_deps(tmp_path: Path) -> None: manifest = load_groups(_manifest(tmp_path)) ordered = resolve(manifest, positional=[], tier="unit") assert set(ordered) == {"unit-a", "unit-b"} + + +def _cross_tier_manifest(tmp_path: Path) -> Path: + p = tmp_path / "groups.yaml" + p.write_text( + """ + version: 1 + groups: + unit-a: + tier: unit + paths: [x] + optional: true + e2e-smoke: + tier: e2e + paths: [x] + requires_platform: true + optional: true + e2e-cross: + tier: e2e + paths: [x] + requires_platform: true + depends_on: [e2e-smoke, unit-a] + optional: true + """ + ) + return p + + +def test_tier_run_does_not_pull_in_other_tiers(tmp_path: Path) -> None: + """Each tier is its own CI leg; a cross-tier dep must not re-run there.""" + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=[], tier="e2e") + assert set(ordered) == {"e2e-smoke", "e2e-cross"} + assert ordered.index("e2e-smoke") < ordered.index("e2e-cross") + + +def test_explicitly_named_group_survives_tier_filter(tmp_path: Path) -> None: + """The filter only drops dep-expanded groups, never an explicit request.""" + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=["unit-a"], tier="e2e") + assert set(ordered) == {"unit-a", "e2e-smoke", "e2e-cross"} + + +def test_cross_tier_dep_still_expands_without_tier_filter(tmp_path: Path) -> None: + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=["e2e-cross"]) + assert set(ordered) == {"unit-a", "e2e-smoke", "e2e-cross"} diff --git a/unstract/sdk1/src/unstract/sdk1/llm.py b/unstract/sdk1/src/unstract/sdk1/llm.py index 1befc96eb3..b0a712b49f 100644 --- a/unstract/sdk1/src/unstract/sdk1/llm.py +++ b/unstract/sdk1/src/unstract/sdk1/llm.py @@ -35,10 +35,6 @@ # Lets tests force a deterministic completion without a provider or a secret. # Unset in production, where this is a no-op. _MOCK_RESPONSE_ENV = "UNSTRACT_LLM_MOCK_RESPONSE" -# Second condition on the hatch, so a stray mock var alone can't fake -# completions and their billing. Deployments that set neither fail closed. -_ENVIRONMENT_ENV = "ENVIRONMENT" -_MOCK_ALLOWED_ENVIRONMENTS = frozenset({"test", "development"}) @lru_cache(maxsize=1) @@ -52,27 +48,10 @@ def _warn_mock_active() -> None: ) -@lru_cache(maxsize=1) -def _warn_mock_refused(environment: str) -> None: - # Loud rather than silent: the var being set at all means someone expected - # mocking, and they need to know why the bill is real. - logger.warning( - "%s is set but %s=%r is not one of %s — calling the provider for real.", - _MOCK_RESPONSE_ENV, - _ENVIRONMENT_ENV, - environment, - sorted(_MOCK_ALLOWED_ENVIRONMENTS), - ) - - def _inject_mock_response(completion_kwargs: dict[str, object]) -> None: mock = os.getenv(_MOCK_RESPONSE_ENV) if not mock or "mock_response" in completion_kwargs: return - environment = os.getenv(_ENVIRONMENT_ENV, "").strip().lower() - if environment not in _MOCK_ALLOWED_ENVIRONMENTS: - _warn_mock_refused(environment) - return _warn_mock_active() completion_kwargs["mock_response"] = mock diff --git a/unstract/sdk1/tests/test_mock_response.py b/unstract/sdk1/tests/test_mock_response.py index 6a4e0b1a59..807d7d05f1 100644 --- a/unstract/sdk1/tests/test_mock_response.py +++ b/unstract/sdk1/tests/test_mock_response.py @@ -38,13 +38,6 @@ def _inject(kwargs: dict[str, object]) -> dict[str, object]: @pytest.fixture(autouse=True) def _reset_warn_cache() -> None: _load_llm_module()._warn_mock_active.cache_clear() - _load_llm_module()._warn_mock_refused.cache_clear() - - -@pytest.fixture(autouse=True) -def _allowed_environment(monkeypatch: pytest.MonkeyPatch) -> None: - """The hatch needs a permitted ENVIRONMENT; the guard itself is tested below.""" - monkeypatch.setenv("ENVIRONMENT", "test") def test_inject_is_noop_when_env_unset(monkeypatch: pytest.MonkeyPatch) -> None: @@ -67,45 +60,6 @@ def test_inject_sets_mock_response_when_env_set( assert _inject({"model": "gpt-4o"})["mock_response"] == "canned answer" -@pytest.mark.parametrize("environment", ["production", "staging", "", " "]) -def test_mock_refused_outside_allowed_environments( - monkeypatch: pytest.MonkeyPatch, environment: str -) -> None: - # The whole point of the guard: a stray mock var in a real deployment must - # not fake completions and their billing. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", environment) - assert _inject({"model": "gpt-4o"}) == {"model": "gpt-4o"} - - -def test_mock_refused_when_environment_unset(monkeypatch: pytest.MonkeyPatch) -> None: - # Production k8s sets no ENVIRONMENT at all, so unset must fail closed. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.delenv("ENVIRONMENT", raising=False) - assert _inject({"model": "gpt-4o"}) == {"model": "gpt-4o"} - - -def test_refusal_is_warned_so_a_real_bill_is_never_silent( - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture -) -> None: - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", "production") - with caplog.at_level("WARNING", logger="unstract.sdk1.llm"): - _inject({"model": "gpt-4o"}) - assert any("not one of" in r.message for r in caplog.records), caplog.text - - -@pytest.mark.parametrize("environment", ["test", "development", "TEST", " Development "]) -def test_mock_applies_in_allowed_environments( - monkeypatch: pytest.MonkeyPatch, environment: str -) -> None: - # `development` is what base compose sets on the workers that run the - # injection; a guard that only accepted `test` would kill the local stack. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", environment) - assert _inject({"model": "gpt-4o"})["mock_response"] == "canned" - - def test_inject_does_not_clobber_explicit_mock_response( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/workers/tests/test_ide_callback.py b/workers/tests/test_ide_callback.py index eb0b2f8c8d..02c5e10b46 100644 --- a/workers/tests/test_ide_callback.py +++ b/workers/tests/test_ide_callback.py @@ -95,6 +95,17 @@ def failure_result(): } +@pytest.fixture +def no_client_plugins(): + """Pin plugin lookup to the OSS answer for callbacks that aren't testing it. + + Left live, a tree with the plugins installed makes a real HTTP call that only + fails after a multi-second connect timeout. + """ + with patch(_PATCH_GET_PLUGIN, return_value=None): + yield + + # --------------------------------------------------------------------------- # TestIdeIndexComplete # --------------------------------------------------------------------------- @@ -346,6 +357,7 @@ def test_exception_does_not_crash(self, mock_get_client, mock_emit_ws, mock_api, # --------------------------------------------------------------------------- +@pytest.mark.usefixtures("no_client_plugins") class TestIdePromptComplete: """Tests for ide_prompt_complete task."""