test: critical e2e test for all features by inur93 · Pull Request #36 · vormadal/vman

inur93 · 2026-02-09T13:40:19Z

No description provided.

- Move dotnet-tools.json to VideoManager/.config/dotnet-tools.json so dotnet tool restore can find the manifest (required for dotnet-ef) - Restore claude.yml and claude-code-review.yml from master (deleted when playwright.yml was replaced with verify.yml) - Add GITHUB_TOKEN to SonarCloud scan step for PR decoration https://claude.ai/code/session_018FEiKhP95yFiPvXg2vZSAy

Copilot

Pull request overview

Adds Playwright E2E coverage for core dashboard flows (items, tagging, tags, collections, navigation), and wires CI to build/lint, run E2E, and perform SonarCloud scanning; also introduces small app/testability tweaks (ARIA labels) and tooling/workflow additions.

Changes:

Add new Playwright spec files + Page Object Models for Tags, Tagging, Items, Collections, and Navigation flows.
Replace the standalone Playwright workflow with a consolidated verify.yml workflow (build/lint + E2E + SonarCloud).
Add SonarCloud project configuration, .NET tool manifest, and a backend csproj glob workaround; add ARIA labels used by E2E selectors.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
video-manager-frontend/tests/tags.spec.ts	New E2E covering tag create/rename/delete with cleanup.
video-manager-frontend/tests/tagging.spec.ts	New E2E covering tagging mode navigation + add/remove tag flow.
video-manager-frontend/tests/navigation.spec.ts	New E2E validating drawer navigation to major pages.
video-manager-frontend/tests/items.spec.ts	New E2E validating items list loading, filtering, and tagging mode navigation.
video-manager-frontend/tests/collections.spec.ts	New E2E covering collection create/detail/collection-mode/delete with cleanup.
video-manager-frontend/tests/helpers/sync.helper.ts	Helper to ensure provider items exist before running item-dependent tests.
video-manager-frontend/tests/pages/tags.page.ts	Page Object for tags CRUD interactions used by E2E.
video-manager-frontend/tests/pages/tagging.page.ts	Page Object for tagging mode interactions and assertions.
video-manager-frontend/tests/pages/navigation.page.ts	Page Object for drawer navigation links/actions.
video-manager-frontend/tests/pages/items.page.ts	Page Object for items listing, filtering, and navigation.
video-manager-frontend/tests/pages/collections.page.ts	Page Object for collections CRUD interactions.
video-manager-frontend/tests/pages/collection-detail.page.ts	Page Object for collection detail behaviors (empty state, mode entry).
video-manager-frontend/src/app/(dashboard)/tags/page.tsx	Adds ARIA labels for edit/delete buttons to support accessible selectors.
sonar-project.properties	Adds SonarCloud configuration (sources/tests/exclusions).
VideoManager/VManBackend/VManBackend.csproj	Adds MSBuild item includes/removes as workaround for SDK glob issue.
VideoManager/.config/dotnet-tools.json	Adds local tool manifest for kiota + dotnet-ef.
.github/workflows/verify.yml	New consolidated CI workflow: build/lint, E2E with backend+db, Sonar scan.
.github/workflows/playwright.yml	Removes prior Playwright CI workflow.
.github/workflows/claude.yml	Adds Claude Code automation workflow triggered by comments/reviews/issues.
.github/workflows/claude-code-review.yml	Adds opt-in Claude Code Review workflow (label-triggered).
.claude/rules/playwright-tests.md	Updates Playwright selector guidance (role → text → test id; avoid xpath).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T13:45:55Z

video-manager-frontend/tests/pages/tagging.page.ts

+    await this.page
+      .locator('.relative.aspect-video')
+      .locator('.absolute')
+      .getByText(tagName, { exact: true })
+      .click();


This method uses brittle class-based CSS selectors (.relative.aspect-video / .absolute) to find overlay tags. This conflicts with the selector convention in .claude/rules/playwright-tests.md (prefer role/text/test id selectors) and is likely to break with styling refactors. Consider adding a stable data-testid/accessible role for the overlay and using getByTestId/getByRole/getByText scoped to that container.

Copilot · 2026-02-09T13:45:55Z

video-manager-frontend/tests/pages/tagging.page.ts

+      this.page
+        .locator('.relative.aspect-video')
+        .locator('.absolute')
+        .getByText(tagName, { exact: true })
+    ).toBeVisible({ timeout: 5000 });


expectTagInOverlay relies on class-based CSS selectors to find the overlay container. Per .claude/rules/playwright-tests.md, prefer role/text/test id selectors; adding a data-testid to the overlay/tag pill would make this assertion much less fragile.

Copilot · 2026-02-09T13:45:55Z

video-manager-frontend/tests/helpers/sync.helper.ts

+  await page.goto('/items');
+  await page.waitForLoadState('domcontentloaded');
+
+  const hasItems = await page
+    .getByText(/showing \d+ of \d+/i)


ensureItemsSynced directly uses Playwright selectors and page interactions. This conflicts with the repo’s Playwright rule in .claude/rules/playwright-tests.md (selectors/interactions should live in Page Objects) and duplicates existing ItemsPage/SyncPage logic. Refactor this helper to use the existing page objects so selectors remain centralized and less brittle.

Copilot · 2026-02-09T13:45:56Z

.github/workflows/verify.yml

+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]


The workflow is restricted to branches: [master], but the repository previously used main/develop in the removed Playwright workflow. If the default branch is still main, this will prevent CI from running on pushes/PRs. Consider removing the branch filter for pull_request, or aligning the branch list with the repo’s default branch configuration.

Copilot · 2026-02-09T13:45:56Z

.github/workflows/verify.yml

+      - name: SonarCloud Scan
+        uses: SonarSource/sonarqube-scan-action@v5
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}


The SonarCloud job runs SonarSource/sonarqube-scan-action while sonar-project.properties includes the C# backend (VideoManager/VManBackend) in sonar.sources. If backend analysis is intended, Sonar typically requires the SonarScanner for .NET (MSBuild integration) with a build between begin/end; the generic scanner often won’t analyze C# correctly. Consider switching to the .NET scanner workflow, or removing the backend path from sonar.sources.

Copilot · 2026-02-09T13:45:56Z

VideoManager/VManBackend/VManBackend.csproj

+  <!-- Workaround for .NET SDK 10.0.102 bug with glob pattern expansion -->
  <ItemGroup>
+    <Compile Remove="**/*.cs" />
+    <Compile Include="**/*.cs" Exclude="bin/**;obj/**" />


PR title suggests the change is only about critical E2E tests, but this PR also includes backend build configuration changes and CI/sonar workflow additions. Consider updating the PR title/description (or splitting) so CI/infra and backend build changes can be reviewed separately from the test additions.

Copilot · 2026-02-09T13:45:56Z

video-manager-frontend/tests/pages/tags.page.ts

+    await this.createTagButton.click();
+    await this.page.locator('#tagName').fill(name);
+    await this.page.getByRole('button', { name: 'Create', exact: true }).click();


This uses a hard-coded #tagName id selector. Since the input has a visible label, prefer getByLabel('Tag Name') (or equivalent) to better match the repo’s selector guidance (role/text/test id) and reduce coupling to DOM ids.

Copilot · 2026-02-09T13:45:56Z

video-manager-frontend/tests/pages/tags.page.ts

+
+    await this.page.locator('#editTagName').fill(newName);
+    await this.page.getByRole('button', { name: 'Rename' }).click();


This uses a hard-coded #editTagName id selector. For consistency with other page objects (e.g., LoginPage, ItemsPage, CollectionsPage) and the selector guidance in .claude/rules/playwright-tests.md, prefer locating this field by its label rather than an element id.

v5 has a known security vulnerability and exits with code 3. GitHub recommends upgrading to v6+. Using v7 (latest) which includes Scanner CLI v8 with embedded Java 21 JRE. https://claude.ai/code/session_018FEiKhP95yFiPvXg2vZSAy

The explicit <Compile Remove="**/*.cs" /> + <Compile Include="**/*.cs" /> workaround for .NET SDK 10.0.102 causes the glob pattern to be passed literally (unexpanded) to CSC on the CI runner, producing: CS2021: File name '**/*.cs' is empty, contains invalid characters The SDK default compile includes already handle **/*.cs automatically, so the explicit Remove/Include is redundant. Keep the Content and EmbeddedResource removals which only filter items (no risk of literal glob being passed to the compiler). https://claude.ai/code/session_018FEiKhP95yFiPvXg2vZSAy

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T15:41:17Z

video-manager-frontend/tests/tags.spec.ts

+    const tagName = `E2E Tag ${Date.now()}`;
+    const renamedTag = `E2E Renamed ${Date.now()}`;
+    let createdTagName: string | null = null;


renamedTag and tagName are both derived from separate Date.now() calls; if they occur within the same millisecond, the rename becomes a no-op and the assertions expecting the old name to disappear will fail (flaky test). Consider generating a single unique suffix once and deriving both names from it (e.g., add -renamed), or use a UUID/random suffix.

Copilot · 2026-02-09T15:41:18Z

video-manager-frontend/tests/pages/items.page.ts

+  }
+
+  async filterByMediaType(type: string) {
+    const mediaTypeSection = this.page.getByRole('heading', { name: /media type/i }).locator('..');


filterByMediaType() scopes to the parent of the "Media Type" heading (locator('..')), but in the Items page markup the badges live in a sibling container (one level higher). As a result, getByText(type) likely won't find the badge and the filter click will fail. Update the locator to target the outer section element (e.g., go up two levels or locate the section by filtering for a container that has the heading) before searching for the badge text.

Suggested change

const mediaTypeSection = this.page.getByRole('heading', { name: /media type/i }).locator('..');

const mediaTypeSection = this.page

.locator('section')

.filter({ has: this.page.getByRole('heading', { name: /media type/i }) });

Copilot · 2026-02-09T15:41:19Z

.github/workflows/verify.yml

+      - name: Start backend
+        working-directory: ./VideoManager/VManBackend
+        run: |
+          dotnet run --no-build &
+          echo $! > backend.pid
+        env:
+          USE_STUB_IMMICH: "true"
+          ASPNETCORE_URLS: "http://localhost:5001"
+          Jwt__SecretKey: "test-secret-key-min-32-characters-long-for-testing-only"
+          Jwt__Issuer: "VManBackend"
+          Jwt__Audience: "VManBackend"
+          Immich__BaseUrl: "http://stub"
+


The backend only seeds the E2E test user when ASPNETCORE_ENVIRONMENT=Development, and it requires TestUser:Email / TestUser:Password configuration values. This workflow doesn't set those, so the Playwright auth setup is likely to fail in CI (no seeded credentials). Set ASPNETCORE_ENVIRONMENT: Development and provide TestUser__Email / TestUser__Password (or equivalent config) in the backend start step (and/or align Playwright TEST_USER_* envs accordingly).

Copilot · 2026-02-09T15:41:19Z

.github/workflows/verify.yml

+          fetch-depth: 0
+
+      - name: SonarCloud Scan
+        uses: SonarSource/sonarqube-scan-action@v7


The workflow uses SonarSource/sonarqube-scan-action with sonar.sources including the .NET backend. The generic scanner typically does not perform C# analysis unless you use the SonarScanner for .NET (begin/build/end) or explicitly exclude the backend. Either switch to the .NET scanner (and build during the scan) or remove the backend from sonar.sources to avoid misleading/partial analysis results.

Suggested change

uses: SonarSource/sonarqube-scan-action@v7

uses: SonarSource/sonarqube-scan-action@v7

with:

args: >

-Dsonar.sources=video-manager-frontend

Copilot · 2026-02-09T15:41:19Z

video-manager-frontend/tests/pages/tagging.page.ts

+      .locator('.relative.aspect-video')
+      .locator('.absolute')
+      .getByText(tagName, { exact: true })
+      .click();
+  }
+
+  async expectTagInOverlay(tagName: string) {
+    await expect(
+      this.page
+        .locator('.relative.aspect-video')
+        .locator('.absolute')
+        .getByText(tagName, { exact: true })


These overlay interactions are located via Tailwind utility class selectors (.relative.aspect-video / .absolute). This is fairly brittle (class names/layout can change without behavior changes). If possible, prefer a role/text/test-id based anchor (e.g., add a data-testid to the tagging overlay container or tag badge list in the app and scope locators to that).

Suggested change

.locator('.relative.aspect-video')

.locator('.absolute')

.getByText(tagName, { exact: true })

.click();

}

async expectTagInOverlay(tagName: string) {

await expect(

this.page

.locator('.relative.aspect-video')

.locator('.absolute')

.getByText(tagName, { exact: true })

.getByRole('button', { name: `Remove tag ${tagName}`, exact: true })

.click();

}

async expectTagInOverlay(tagName: string) {

await expect(

this.page.getByRole('button', {

name: `Remove tag ${tagName}`,

exact: true,

})

Copilot · 2026-02-09T15:41:19Z

video-manager-frontend/tests/tagging.spec.ts

+    // Create and add a tag to current item
+    const tagName = `E2E Tagging ${Date.now()}`;
+    await taggingPage.createAndAddTag(tagName);
+
+    // Verify tag appears on the item overlay
+    await taggingPage.expectTagInOverlay(tagName);
+
+    // Remove the tag from overlay
+    await taggingPage.removeTagFromOverlay(tagName);
+
+    // Verify tag moved to available tags list
+    await taggingPage.expectTagInAvailableList(tagName);
+


This test creates a new tag but never deletes it, so repeated E2E runs will leave behind accumulating tags and can eventually slow down or destabilize the suite (and may affect assertions that depend on tag lists). Consider adding a finally cleanup that navigates to the Tags page and deletes the created tag (or use an API/db cleanup hook).

Copilot AI review requested due to automatic review settings February 9, 2026 13:40

Copilot started reviewing on behalf of inur93 February 9, 2026 13:40 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

claude added 2 commits February 9, 2026 13:48

fix: upgrade sonarqube-scan-action from v5 to v7

46d5911

v5 has a known security vulnerability and exits with code 3. GitHub recommends upgrading to v6+. Using v7 (latest) which includes Scanner CLI v8 with embedded Java 21 JRE. https://claude.ai/code/session_018FEiKhP95yFiPvXg2vZSAy

Copilot AI review requested due to automatic review settings February 9, 2026 15:34

Copilot started reviewing on behalf of inur93 February 9, 2026 15:34 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

inur93 changed the base branch from master to feature/e2e-tests February 9, 2026 19:25

inur93 merged commit f8cf519 into feature/e2e-tests Feb 9, 2026
13 of 18 checks passed

inur93 deleted the claude/fix-ci-workflows-GcHbF branch February 9, 2026 19:31


		await this.page.locator('#editTagName').fill(newName);
		await this.page.getByRole('button', { name: 'Rename' }).click();

Comments

Conversation

inur93 commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants