fix tests on localhost #469

tomusdrw · 2025-12-30T22:51:37Z

No description provided.

netlify · 2025-12-30T22:51:42Z

✅ Deploy Preview for pvm-debugger ready!

Name	Link
🔨 Latest commit	`929e1e0`
🔍 Latest deploy log	https://app.netlify.com/projects/pvm-debugger/deploys/6954577c815528000844f7b8
😎 Deploy Preview	https://deploy-preview-469--pvm-debugger.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

coderabbitai · 2025-12-30T22:51:46Z

📝 Walkthrough

Walkthrough

The changes standardize test data attributes from test-id to data-testid across multiple components, update test selectors to use Playwright's getByTestId() method, enable the webServer configuration in Playwright, and replace fixed sleep statements with explicit waits in tests.

Changes

Cohort / File(s)	Summary
Test Configuration `playwright.config.ts`	Activates webServer block to run build and start Vite preview server on port 5173 before tests with 120-second timeout; conditionally reuses existing server based on CI environment.
Component Test Attributes `src/components/Instructions/InstructionItem.tsx`, `src/components/MemoryPreview/MemoryInfinite.tsx`, `src/components/Registers/index.tsx`	Standardizes test data attributes from `test-id` or `data-test-id` to `data-testid` for Playwright test selectors; no structural or behavioral changes.
Test Selector & Wait Updates `tests/memory-range.spec.ts`, `tests/run-program.spec.ts`	Updates element selectors to use `getByTestId()` instead of attribute-based locators; replaces fixed sleep after "Run" with explicit wait for "HALT" status (15-second timeout) and adds 2-second initialization delay before interactions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset.	Add a pull request description explaining the motivation and details of the changes, such as why test selectors needed updating and how the localhost setup improvements benefit the test suite.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title 'fix tests on localhost' is somewhat vague and generic, using the non-descriptive term 'fix' without specifying what aspect of tests is being fixed or why localhost is relevant.	Consider a more specific title like 'Update test selectors and add explicit waits for localhost testing' or 'Replace test-id attributes with data-testid and improve test timing' to better convey the actual changes.

✨ Finishing touches

📝 Generate docstrings

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/memory-range.spec.ts (1)
9-10: Consider replacing fixed waits with explicit state checks.

While the 2-second waits work for localhost, explicit waits for observable program states (like checking for initialization status or first instruction visibility) would be more reliable and potentially faster, similar to the approach in tests/run-program.spec.ts (lines 15-17).
Example: Wait for observable state instead of fixed timeout
- // Wait for program to load and initialize
- await page.waitForTimeout(2000);
+ // Wait for program to initialize by checking for visible instructions
+ await expect(page.locator('a:has-text("STORE_U16")')).toBeVisible({ timeout: 5000 });
Alternatively, wait for a specific program status or memory initialization indicator if available.
Also applies to: 31-32, 51-52, 73-74

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f55dff and 929e1e0.

📒 Files selected for processing (6)

playwright.config.ts
src/components/Instructions/InstructionItem.tsx
src/components/MemoryPreview/MemoryInfinite.tsx
src/components/Registers/index.tsx
tests/memory-range.spec.ts
tests/run-program.spec.ts

🧰 Additional context used

📓 Path-based instructions (5)

**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.spec.{ts,tsx}: Unit specs should use the *.spec.ts(x) suffix and stay beside their modules
Match test file names to the unit under test (e.g., store/pvmSlice.spec.ts)

Files:

tests/run-program.spec.ts
tests/memory-range.spec.ts

tests/**/*.spec.ts

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.spec.ts: Playwright specs in tests/ should script critical journeys such as uploading WASM or stepping instructions
Reuse fixtures from tests/utils for deterministic Playwright inputs

Files:

tests/run-program.spec.ts
tests/memory-range.spec.ts

src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/components/**/*.{ts,tsx}: Keep reusable UI components in src/components/
Write TypeScript-first functional components

Files:

src/components/Instructions/InstructionItem.tsx
src/components/MemoryPreview/MemoryInfinite.tsx
src/components/Registers/index.tsx

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Use two-space indentation in TypeScript files
Use descriptive filenames in kebab-case (e.g., memory-range-panel.tsx)
Validate external inputs with Zod when parsing user data

Files:

src/components/Instructions/InstructionItem.tsx
src/components/MemoryPreview/MemoryInfinite.tsx
src/components/Registers/index.tsx

src/**/*.{css,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Favor Tailwind utilities from globals.css before adding new CSS files

Files:

src/components/Instructions/InstructionItem.tsx
src/components/MemoryPreview/MemoryInfinite.tsx
src/components/Registers/index.tsx

🧠 Learnings (8)

📓 Common learnings

Learnt from: CR
Repo: FluffyLabs/pvm-debugger PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:06:57.521Z
Learning: UI tweaks should update the affected Playwright expectations

Learnt from: CR
Repo: FluffyLabs/pvm-debugger PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:06:57.521Z
Learning: Applies to tests/**/*.spec.ts : Reuse fixtures from `tests/utils` for deterministic Playwright inputs

Learnt from: CR
Repo: FluffyLabs/pvm-debugger PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:06:57.521Z
Learning: Applies to tests/**/*.spec.ts : Playwright specs in `tests/` should script critical journeys such as uploading WASM or stepping instructions

Learnt from: CR
Repo: FluffyLabs/pvm-debugger PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:06:57.521Z
Learning: Applies to src/**/*.spec.{ts,tsx} : Vitest with Testing Library is configured through `src/test-setup.ts`; assert observable behavior