Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
name: Run tests

permissions:
contents: read

on:
workflow_call:
pull_request:
branches:
- main
- '!dependabot/**'
paths:
- src/**
- tests/**
- '!docs/**'
- package.json
- package-lock.json
- svelte.config.*
- vite.config.*
- playwright.config.*
- tsconfig*.json
- .github/workflows/run-tests.yml
Comment on lines +12 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add pnpm-lock.yaml to the workflow path filters.
Dependency-only changes to the pnpm lockfile won’t trigger this workflow, so tests can be skipped.

🔧 Proposed fix
         paths:
             - src/**
             - tests/**
             - '!docs/**'
             - package.json
+            - pnpm-lock.yaml
             - package-lock.json
             - svelte.config.*
             - vite.config.*
             - playwright.config.*
             - tsconfig*.json
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
paths:
- src/**
- tests/**
- '!docs/**'
- package.json
- package-lock.json
- svelte.config.*
- vite.config.*
- playwright.config.*
- tsconfig*.json
- .github/workflows/run-tests.yml
paths:
- src/**
- tests/**
- '!docs/**'
- package.json
- pnpm-lock.yaml
- package-lock.json
- svelte.config.*
- vite.config.*
- playwright.config.*
- tsconfig*.json
- .github/workflows/run-tests.yml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/run-tests.yml around lines 12 - 22, The workflow's path
filters under the paths: list are missing pnpm-lock.yaml so changes to the pnpm
lockfile won't trigger the job; update the paths: entries to include
"pnpm-lock.yaml" alongside package.json and package-lock.json so lockfile-only
changes kick off the workflow (modify the paths: section in the run-tests.yml
workflow to add pnpm-lock.yaml).


concurrency:
group: run-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
unit-tests:
runs-on: blacksmith-2vcpu-ubuntu-2404 # trunk-ignore(actionlint/runner-label)
timeout-minutes: 10
steps:
- name: Checkout
uses: actions/checkout@v6 # zizmor: ignore[unpinned-uses]
with:
persist-credentials: false
fetch-depth: 1

- uses: pnpm/action-setup@v4 # zizmor: ignore[unpinned-uses]
with:
version: 10

- name: Use Node.js - 24
uses: actions/setup-node@v6 # zizmor: ignore[unpinned-uses]
with:
node-version: 24

- name: Cache build artifacts
uses: actions/cache@v5 # zizmor: ignore[unpinned-uses]
with:
path: |
.svelte-kit
dist
key: ${{ runner.os }}-build-${{ hashFiles('src/**/*', 'pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-build-

- name: Cache vitest artifacts
uses: actions/cache@v5 # zizmor: ignore[unpinned-uses]
with:
path: |
node_modules/.vitest
test-results
key: ${{ runner.os }}-vitest-${{ hashFiles('pnpm-lock.yaml', 'src/**/*.{test,spec}.{js,ts}', 'tests/**/*.{test,spec}.{js,ts}', 'vitest.config.ts') }}
restore-keys: |
${{ runner.os }}-vitest-

- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Run unit tests
run: |
pnpm build
pnpm test

- name: Upload unit test results
if: always()
uses: actions/upload-artifact@v4 # zizmor: ignore[unpinned-uses]
with:
name: unit-test-results
path: |
coverage/**
test-results/**
if-no-files-found: ignore
retention-days: 7

e2e-tests:
needs: unit-tests
runs-on: blacksmith-2vcpu-ubuntu-2404 # trunk-ignore(actionlint/runner-label)
timeout-minutes: 25
strategy:
fail-fast: false
matrix:
include:
- shard: 1/2
shard_name: 1-2
- shard: 2/2
shard_name: 2-2
steps:
- name: Checkout
uses: actions/checkout@v6 # zizmor: ignore[unpinned-uses]
with:
persist-credentials: false
fetch-depth: 1

- uses: pnpm/action-setup@v4 # zizmor: ignore[unpinned-uses]
with:
version: 10

- name: Use Node.js - 24
uses: actions/setup-node@v6 # zizmor: ignore[unpinned-uses]
with:
node-version: 24

- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Cache Playwright browsers
uses: actions/cache@v5 # zizmor: ignore[unpinned-uses]
with:
path: ~/.cache/ms-playwright
key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-playwright-

- name: Install Playwright browsers
run: pnpm exec playwright install --with-deps

- name: Sanity check - list discovered tests
run: pnpm exec playwright test --list | head -n 50 | cat

- name: Run e2e tests (sharded)
env:
CI: true
run: pnpm exec playwright test --config=playwright.config.ts --shard=${{ matrix.shard }} --reporter=line

Comment on lines +115 to +136
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "playwright.config.ts" -o -name "playwright.config.js" | head -5

Repository: humanspeak/svelte-diff-match-patch

Length of output: 100


🏁 Script executed:

find . -name "package.json" | head -3

Repository: humanspeak/svelte-diff-match-patch

Length of output: 112


🏁 Script executed:

cat -n playwright.config.ts

Repository: humanspeak/svelte-diff-match-patch

Length of output: 1395


🏁 Script executed:

cat -n package.json

Repository: humanspeak/svelte-diff-match-patch

Length of output: 5017


Build step is required and missing.
The playwright.config.ts webServer runs npm run build && npm run preview on startup. Without a build step in the workflow, the dist directory won't exist and tests will fail. Add pnpm build before the e2e tests.

Additionally, playwright.config.ts uses npm run but the workflow uses pnpm—consider updating the webServer command to pnpm build && pnpm preview for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/run-tests.yml around lines 115 - 136, The workflow is
missing the build step required by the Playwright webServer in
playwright.config.ts; add a step that runs "pnpm build" (before the "Run e2e
tests (sharded)" step) so the dist directory exists for tests, and ensure the
webServer command in playwright.config.ts is consistent with the package manager
by changing its startup command from "npm run build && npm run preview" to "pnpm
build && pnpm preview" (or equivalent) so builds and previews use the same
package manager.

- name: Upload Playwright artifacts
if: always()
uses: actions/upload-artifact@v4 # zizmor: ignore[unpinned-uses]
with:
name: playwright-artifacts-${{ matrix.shard_name }}
path: |
playwright-report/**
test-results/**
blob-report/**
if-no-files-found: ignore
retention-days: 7
3 changes: 3 additions & 0 deletions .trunk/trunk.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ lint:
- linters: [markdownlint]
paths:
- .changeset/**
- linters: [eslint]
paths:
- '**/*.svelte.ts'
definitions:
- name: prettier
files:
Expand Down
8 changes: 4 additions & 4 deletions tests/default.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ test.describe('SvelteDiffMatchPatch', () => {
})

test('renders a visible diff', async ({ page }) => {
await page.fill('[data-testid="text1"]', 'hello world')
await page.fill('[data-testid="text2"]', 'hello brave world')
await page.getByTestId('text1').fill('hello world')
await page.getByTestId('text2').fill('hello brave world')
await expect(page.getByTestId('diff-result')).toContainText('brave')
await expect(page.getByTestId('diff-result')).toContainText('hello')
await expect(page.getByTestId('diff-result')).toContainText('world')
})

test('applies custom rendererClasses', async ({ page }) => {
await page.fill('[data-testid="text1"]', 'foo shoo')
await page.fill('[data-testid="text2"]', 'bar shoo')
await page.getByTestId('text1').fill('foo shoo')
await page.getByTestId('text2').fill('bar shoo')
await expect(page.getByTestId('diff-result').locator('.diff-remove')).toBeVisible()
await expect(page.getByTestId('diff-result').locator('.diff-insert')).toBeVisible()
await expect(page.getByTestId('diff-result').locator('.diff-equal')).toBeVisible()
Expand Down
11 changes: 3 additions & 8 deletions tests/expected-patterns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,10 @@ test.describe('Expected Patterns', () => {
})

test('non-captured deviations still show as insert/remove', async ({ page }) => {
// Use a simple template where we can have both expected captures and deviations
await page.getByTestId('text1').fill('Hello (?<name>.+) end')
await page.getByTestId('text2').fill('Goodbye World end')
// "World" is captured as expected, but "Hello" vs "Goodbye" is a real deviation
// Actually regex won't match because "Hello" != "Goodbye" literal...
// Use: template matches but has surrounding diffs from diff algorithm
await page.getByTestId('text1').fill('(?<name>.+) likes coding')
// Use \w+ instead of .+ so the capture is constrained to "Jason" only,
// leaving "coding" vs "hacking" as a real deviation for insert/remove
await page.getByTestId('text1').fill('(?<name>\\w+) likes coding')
await page.getByTestId('text2').fill('Jason likes hacking')
// "Jason" is captured, "coding" vs "hacking" is a real deviation
await expect(
page.getByTestId('diff-result').locator('.diff-expected').first()
).toBeVisible()
Expand Down
4 changes: 2 additions & 2 deletions tests/snippets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ test.describe('SvelteDiffMatchPatch', () => {
})

test('applies custom rendererClasses', async ({ page }) => {
await page.fill('[data-testid="text1"]', 'foo shoo')
await page.fill('[data-testid="text2"]', 'bar shoo')
await page.getByTestId('text1').fill('foo shoo')
await page.getByTestId('text2').fill('bar shoo')
await expect(page.getByTestId('diff-result').locator('.diff-remove')).toBeVisible()
await expect(page.getByTestId('diff-result').locator('.diff-insert')).toBeVisible()
await expect(page.getByTestId('diff-result').locator('.diff-equal')).toBeVisible()
Expand Down
Loading