From 49686a746ad84312e883ed17d4a63ee1a6beea9a Mon Sep 17 00:00:00 2001 From: rookiestar28 <151893693+rookiestar28@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:03:23 +0800 Subject: [PATCH] finalize R53/R54/R57 reliability (hot-reload apply semantics, UI guards, model precedence) --- scripts/run_unittests.py | 2 +- tests/TEST_SOP.md | 51 ++++++++++ tests/e2e/specs/settings.spec.js | 97 +++++++++++++++++++ tests/e2e/test-harness.html | 2 +- ...3_apply.py => test_r53_apply_semantics.py} | 0 ...7_precedence.py => test_r57_precedence.py} | 0 6 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/specs/settings.spec.js rename tests/{repro_r53_apply.py => test_r53_apply_semantics.py} (100%) rename tests/{repro_r57_precedence.py => test_r57_precedence.py} (100%) diff --git a/scripts/run_unittests.py b/scripts/run_unittests.py index 6e56562..17fef38 100644 --- a/scripts/run_unittests.py +++ b/scripts/run_unittests.py @@ -9,10 +9,10 @@ from __future__ import annotations import argparse import io import os -from pathlib import Path import sys import time import unittest +from pathlib import Path def _build_suite(args: argparse.Namespace) -> unittest.TestSuite: diff --git a/tests/TEST_SOP.md b/tests/TEST_SOP.md index 6ac2397..b1cb8ff 100644 --- a/tests/TEST_SOP.md +++ b/tests/TEST_SOP.md @@ -13,6 +13,20 @@ Every implementation plan must include the **full test validation procedure** in - `pre-commit` installed: `python -m pip install pre-commit` - Frontend deps installed: `npm install` +## Environment Sanity (Required Guardrails) + +- **Python interpreter must be consistent** for all test commands. + - Verify: `python -c "import sys; print(sys.executable)"` + - If you use conda or venv, ensure the same interpreter runs unit tests and connector tests. +- **Project venv recommended**: use `.venv` when possible to avoid mixed dependencies. + - Create: `python -m venv .venv` + - Activate (bash): `source .venv/bin/activate` + - Activate (pwsh): `.\.venv\Scripts\Activate.ps1` + - If tests fail due to missing deps in CI parity, **rerun in `.venv` and record that in the implementation record**. +- **Node version must be 18+** before E2E: + - Verify: `node -v` + - If mismatch in WSL, use the Node 18 path specified below. + ## Environment Parity Guardrails (CI Safety) To avoid local vs CI mismatches: @@ -22,6 +36,43 @@ To avoid local vs CI mismatches: - If a test truly requires an optional dependency, mark it with a **clear skip** when the dep is unavailable. - Record the environment in the implementation record (OS, Python, Node, and any extras installed) so mismatches are visible. +## Offline / Restricted Network Pre-commit (Fail Fast) + +If your environment cannot reach GitHub, `pre-commit` may hang while installing hook repos. +Use **one** of the following, and record it in the implementation record: + +1) **Preferred**: run once with network to populate the cache + - `pre-commit install --install-hooks` + - Subsequent runs will use cache without network. +2) **Proxy**: configure `https_proxy` / `http_proxy` for GitHub access. +3) **Fail-fast guard**: if GitHub access is blocked, stop and fix connectivity or use cached hooks. + - Do not mark pre-commit as "passed" unless the hooks complete successfully. + +Do **not** switch hooks to `repo: local` unless CI is updated to match, or you will reintroduce local/CI divergence. + +## Pre-commit Cache Repair (If Cache Is Corrupt) + +Symptoms: +- `InvalidManifestError` or missing `.pre-commit-hooks.yaml` +- partial venv in pre-commit cache +- repeated install failures even after network is restored + +Fix (choose one): + +1) **Clear cache and re-install hooks (recommended)** + - Linux/WSL: + - `rm -rf ~/.cache/pre-commit` + - `pre-commit install --install-hooks` + - Windows (PowerShell): + - `Remove-Item -Recurse -Force \"$env:USERPROFILE\\.cache\\pre-commit\"` + - `pre-commit install --install-hooks` + +2) **Set a clean cache location** + - `set PRE_COMMIT_HOME=/path/to/new/cache` + - `pre-commit install --install-hooks` + +If GitHub is unreachable, the above will still fail; fix connectivity or configure a proxy first. + ## Required Pre-Push Workflow (Must Run) ### Optional automation (recommended) diff --git a/tests/e2e/specs/settings.spec.js b/tests/e2e/specs/settings.spec.js new file mode 100644 index 0000000..d2b173c --- /dev/null +++ b/tests/e2e/specs/settings.spec.js @@ -0,0 +1,97 @@ +import { test, expect } from '@playwright/test'; +import { mockComfyUiCore, waitForMoltbotReady, clickTab } from '../utils/helpers.js'; + +test.describe('Settings Tab Stability', () => { + test.beforeEach(async ({ page }) => { + await mockComfyUiCore(page); + + // Mock Config GET & PUT + await page.route('**/openclaw/config', async (route) => { + if (route.request().method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + ok: true, + config: { + provider: 'openai', + model: 'gpt-4o', + base_url: '', + timeout_sec: 120, + max_retries: 3 + }, + sources: { provider: 'default' }, + providers: [ + { id: 'openai', label: 'OpenAI' }, + { id: 'anthropic', label: 'Anthropic' }, + { id: 'custom', label: 'Custom' } + ], + apply: {} + }), + }); + } else if (route.request().method() === 'PUT') { + // Mock Config PUT (R53 feedback) + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + ok: true, + apply: { + applied_now: ['provider', 'model'], + restart_required: [] + } + }), + }); + } + }); + + // Mock Logs (Dependency) + await page.route('**/openclaw/logs/tail*', async (route) => { + await route.fulfill({ status: 200, body: JSON.stringify({ ok: true, content: [] }) }); + }); + // Mock Health (Dependency) + // Note: harness mock for health is overridden by page.route if this line executes? + // Actually, harness uses window.fetch. Mocking window.fetch happens in harness. + // If we want to support config in health, we modified harness directly. + // So this line is REDUNDANT or IGNORED for calls from UI? + // But good to keep for any network fallbacks. + await page.route('**/openclaw/health', async (route) => { + await route.fulfill({ status: 200, body: JSON.stringify({ ok: true, config: { llm_key_configured: true }, pack: { version: 'test' } }) }); + }); + + await page.goto('test-harness.html'); + await waitForMoltbotReady(page); + }); + + test('loads settings without flicker and populates fields', async ({ page }) => { + await clickTab(page, 'Settings'); + + // Check for specific fields to ensure render complete + // We expect the provider select to be 'openai' + const providerSelect = page.locator('select').first(); + // Wait for it to be visible to ensure "Loading..." is gone + await expect(providerSelect).toBeVisible(); + await expect(providerSelect).toHaveValue('openai'); + + // Model input should match + // Note: The UI has a model select and input. The input is default visible. + // Use first visible text input in settings tab logic (approximate but robust enough) + const modelInput = page.locator('input[type="text"]').first(); + await expect(modelInput).toBeVisible(); + await expect(modelInput).toHaveValue('gpt-4o'); + + // Ensure no 404 warning + await expect(page.locator('text=Backend 404')).not.toBeVisible(); + }); + + test('save triggers hot-reload feedback (R53)', async ({ page }) => { + await clickTab(page, 'Settings'); + + // Click Save (exact match to avoid "Save Key") + await page.getByRole('button', { name: 'Save', exact: true }).click(); + + // Expect success message + await expect(page.locator('.moltbot-status.ok')).toContainText('Saved!'); + await expect(page.locator('.moltbot-status.ok')).toContainText('Applied immediately'); + }); +}); diff --git a/tests/e2e/test-harness.html b/tests/e2e/test-harness.html index 0e28799..e86af29 100644 --- a/tests/e2e/test-harness.html +++ b/tests/e2e/test-harness.html @@ -56,7 +56,7 @@ url.endsWith('/openclaw/health') || url.endsWith('/api/openclaw/health') || url.endsWith('/moltbot/health') || url.endsWith('/api/moltbot/health') ) { - return new Response(JSON.stringify({ ok: true, pack: { name: 'ComfyUI-OpenClaw', version: 'test' }, access_policy: { observability: 'loopback_only', token_configured: false } }), { + return new Response(JSON.stringify({ ok: true, pack: { name: 'ComfyUI-OpenClaw', version: 'test' }, access_policy: { observability: 'loopback_only', token_configured: false }, config: {} }), { status: 200, headers: { 'Content-Type': 'application/json' }, }); diff --git a/tests/repro_r53_apply.py b/tests/test_r53_apply_semantics.py similarity index 100% rename from tests/repro_r53_apply.py rename to tests/test_r53_apply_semantics.py diff --git a/tests/repro_r57_precedence.py b/tests/test_r57_precedence.py similarity index 100% rename from tests/repro_r57_precedence.py rename to tests/test_r57_precedence.py