chore: Add some tests #283

Merged

jank merged 2 commits from playwright into main 2025-06-06 11:59:12 +00:00

Conversation 13 Commits 2 Files changed 12 +216 -7

jank commented 2025-06-04 09:27:24 +00:00

Owner

Copy link

No description provided.

jank added 1 commit 2025-06-04 09:27:24 +00:00

chore: Add some tests 9fe210a5b9

jank changed title from chore: Add some tests to WIP: chore: Add some tests 2025-06-04 09:27:27 +00:00

forgejo-actions added the

frontend

size

small

labels 2025-06-04 09:27:38 +00:00

Claude commented 2025-06-04 09:28:25 +00:00

Collaborator

Copy link

Code Review

✅ Positive aspects:

• Good addition of Playwright e2e testing framework
• Proper dependency management and configuration
• Appropriate .gitignore additions for test artifacts

❌ Issues to address:

frontend/playwright.config.ts:37

• Missing semicolon at end of export statement

frontend/e2e/homepage.spec.ts:7-8

• Hardcoded German text ('Willkommensbonus', 'von bis zu €') makes tests language-dependent and fragile
• Consider using data-testid attributes or more robust selectors

💡 Suggestions:

• Add test scripts to package.json (e.g., "test:e2e": "playwright test")
• Consider using data-testid attributes for more reliable element selection
• Add brief documentation on running e2e tests
• Remove WIP status if this is ready for review

The Playwright setup is solid, but the test implementation could be more robust.

## Code Review ### ✅ Positive aspects: - Good addition of Playwright e2e testing framework - Proper dependency management and configuration - Appropriate .gitignore additions for test artifacts ### ❌ Issues to address: **frontend/playwright.config.ts:37** - Missing semicolon at end of export statement **frontend/e2e/homepage.spec.ts:7-8** - Hardcoded German text ('Willkommensbonus', 'von bis zu €') makes tests language-dependent and fragile - Consider using data-testid attributes or more robust selectors ### 💡 Suggestions: - Add test scripts to package.json (e.g., "test:e2e": "playwright test") - Consider using data-testid attributes for more reliable element selection - Add brief documentation on running e2e tests - Remove WIP status if this is ready for review The Playwright setup is solid, but the test implementation could be more robust.

jank force-pushed playwright from 9fe210a5b9 to ff03649ce7 2025-06-04 09:32:26 +00:00 Compare

forgejo-actions added

ci

size

medium

and removed

size

small

labels 2025-06-04 09:32:40 +00:00

Claude commented 2025-06-04 09:35:38 +00:00

Collaborator

Copy link

Code Review: Found several issues that need fixing:

1. CI Config (.gitea/workflows/ci.yml:190-193): Missing working-directory for Playwright install step, and should use bunx instead of npx for consistency
2. Playwright config (playwright.config.ts:37): Missing semicolon - should be });
3. Test file (e2e/homepage.spec.ts:7-8): Fragile assertions using German text that will break if content changes. Use data-testid instead
4. Package.json: Unnecessary reordering of devDependencies

Please fix these issues before merging. The E2E testing addition is valuable but needs these corrections.

Code Review: Found several issues that need fixing: 1. CI Config (.gitea/workflows/ci.yml:190-193): Missing working-directory for Playwright install step, and should use bunx instead of npx for consistency 2. Playwright config (playwright.config.ts:37): Missing semicolon - should be }); 3. Test file (e2e/homepage.spec.ts:7-8): Fragile assertions using German text that will break if content changes. Use data-testid instead 4. Package.json: Unnecessary reordering of devDependencies Please fix these issues before merging. The E2E testing addition is valuable but needs these corrections.

jank force-pushed playwright from ff03649ce7 to 62999d749d 2025-06-04 09:42:23 +00:00 Compare

Claude commented 2025-06-04 09:43:38 +00:00

Collaborator

Copy link

Code Review Feedback:

The PR adds valuable E2E testing infrastructure, but there are several issues to address:

Critical Issues:

1. **** - Playwright commands should run from directory. Add to these steps.

2. **** - Missing semicolon. Should be not

Improvements Needed:
3. **** - Test assertions are fragile as they depend on German text ('Willkommensbonus', 'von bis zu €'). Consider using data-testid attributes or more semantic selectors.

4. **** - Unnecessary reordering of creates noise in the diff.

Good additions:

• Proper Playwright setup and configuration
• Appropriate CI integration
• Correct dependency management

Please fix the working directory and syntax issues before merging.

**Code Review Feedback:** The PR adds valuable E2E testing infrastructure, but there are several issues to address: **Critical Issues:** 1. **** - Playwright commands should run from directory. Add to these steps. 2. **** - Missing semicolon. Should be not **Improvements Needed:** 3. **** - Test assertions are fragile as they depend on German text ('Willkommensbonus', 'von bis zu €'). Consider using data-testid attributes or more semantic selectors. 4. **** - Unnecessary reordering of creates noise in the diff. **Good additions:** - Proper Playwright setup and configuration - Appropriate CI integration - Correct dependency management Please fix the working directory and syntax issues before merging.

Claude commented 2025-06-04 09:43:47 +00:00

Collaborator

Copy link

Code Review Feedback: The PR adds valuable E2E testing infrastructure, but there are several issues to address. Critical Issues: 1. .gitea/workflows/ci.yml lines 27-28 - Playwright commands should run from frontend/ directory. Add working-directory: ./frontend to these steps. 2. frontend/playwright.config.ts line 37 - Missing semicolon. Should be }); not }). Improvements Needed: 3. frontend/e2e/homepage.spec.ts lines 6-8 - Test assertions are fragile as they depend on German text. Consider using data-testid attributes. 4. frontend/package.json lines 42-44 - Unnecessary reordering creates noise in diff. Good additions: Proper Playwright setup, CI integration, correct dependency management. Please fix the working directory and syntax issues before merging.

Code Review Feedback: The PR adds valuable E2E testing infrastructure, but there are several issues to address. Critical Issues: 1. .gitea/workflows/ci.yml lines 27-28 - Playwright commands should run from frontend/ directory. Add working-directory: ./frontend to these steps. 2. frontend/playwright.config.ts line 37 - Missing semicolon. Should be }); not }). Improvements Needed: 3. frontend/e2e/homepage.spec.ts lines 6-8 - Test assertions are fragile as they depend on German text. Consider using data-testid attributes. 4. frontend/package.json lines 42-44 - Unnecessary reordering creates noise in diff. Good additions: Proper Playwright setup, CI integration, correct dependency management. Please fix the working directory and syntax issues before merging.

jank force-pushed playwright from 62999d749d to 7ea69571bc 2025-06-04 09:45:13 +00:00 Compare

jank force-pushed playwright from 7ea69571bc to a9064f2e44 2025-06-04 09:48:02 +00:00 Compare

Claude commented 2025-06-04 09:48:20 +00:00

Collaborator

Copy link

Code Review: Issues to address: 1) CI config line 190 missing working directory - should cd frontend before bunx playwright test, 2) playwright.config.ts missing trailing newline at line 37, 3) package.json devDependencies not alphabetically ordered after moving @angular/compiler-cli, 4) Test assertions in homepage.spec.ts lines 6-8 are fragile using text like 'von bis zu €' - consider data-testid selectors instead. Overall good Playwright setup with proper browser coverage and CI integration.

Code Review: Issues to address: 1) CI config line 190 missing working directory - should cd frontend before bunx playwright test, 2) playwright.config.ts missing trailing newline at line 37, 3) package.json devDependencies not alphabetically ordered after moving @angular/compiler-cli, 4) Test assertions in homepage.spec.ts lines 6-8 are fragile using text like 'von bis zu €' - consider data-testid selectors instead. Overall good Playwright setup with proper browser coverage and CI integration.

Claude commented 2025-06-04 09:49:03 +00:00

Collaborator

Copy link

Code Review for PR #283: Add Playwright E2E Testing

Overall, this is a good addition that sets up end-to-end testing infrastructure for the frontend. The implementation is mostly solid, but there are a few minor issues to address:

✅ Positives:

• Good CI integration with proper conditional execution based on file changes
• Appropriate caching strategy for node_modules in CI
• Reasonable Playwright configuration with multi-browser support
• Proper gitignore additions for test artifacts

🔧 Issues to Address:

playwright.config.ts:37 - Missing semicolon at the end of the file. The export statement should end with a semicolon for consistency.

frontend/e2e/homepage.spec.ts:7-8 - The test assertions are quite brittle as they depend on specific German text ('Willkommensbonus', 'von bis zu €'). Consider using data-testid attributes or more generic selectors that won't break if text content changes.

frontend/package.json - The devDependencies reordering (moving @angular/compiler-cli) seems unnecessary and adds noise to the diff.

💡 Suggestions:

• Consider adding a timeout configuration in playwright.config.ts for slower CI environments
• The homepage test could benefit from more comprehensive assertions (e.g., checking for navigation elements, game cards)
• Document the new test commands in CLAUDE.md or README

Minor:

The CI job looks well-configured with appropriate dependency caching and conditional execution. Good work on the infrastructure setup!

## Code Review for PR #283: Add Playwright E2E Testing Overall, this is a good addition that sets up end-to-end testing infrastructure for the frontend. The implementation is mostly solid, but there are a few minor issues to address: ### ✅ Positives: - Good CI integration with proper conditional execution based on file changes - Appropriate caching strategy for node_modules in CI - Reasonable Playwright configuration with multi-browser support - Proper gitignore additions for test artifacts ### 🔧 Issues to Address: **playwright.config.ts:37** - Missing semicolon at the end of the file. The export statement should end with a semicolon for consistency. **frontend/e2e/homepage.spec.ts:7-8** - The test assertions are quite brittle as they depend on specific German text ('Willkommensbonus', 'von bis zu €'). Consider using data-testid attributes or more generic selectors that won't break if text content changes. **frontend/package.json** - The devDependencies reordering (moving @angular/compiler-cli) seems unnecessary and adds noise to the diff. ### 💡 Suggestions: - Consider adding a timeout configuration in playwright.config.ts for slower CI environments - The homepage test could benefit from more comprehensive assertions (e.g., checking for navigation elements, game cards) - Document the new test commands in CLAUDE.md or README ### Minor: The CI job looks well-configured with appropriate dependency caching and conditional execution. Good work on the infrastructure setup\!

jank force-pushed playwright from a9064f2e44 to 4c9bd97a49 2025-06-04 09:50:22 +00:00 Compare

Claude commented 2025-06-04 09:51:41 +00:00

Collaborator

Copy link

Code Review Results

This PR adds Playwright E2E testing infrastructure, which is a valuable addition. However, there are several minor issues that should be addressed:

Issues to Fix:

1. Missing trailing newlines in multiple files:

2. Package.json ordering ():

   • should maintain alphabetical order in devDependencies

3. Fragile test selectors ():

   • Text-based assertions like and are brittle and language-dependent
   • Consider using data-testid attributes or more stable selectors

Recommendations:

• Add trailing newlines to maintain consistency
• Use more robust E2E test selectors for better maintainability
• Consider testing functionality rather than specific text content

The Playwright configuration and CI integration look solid overall.

**Code Review Results** This PR adds Playwright E2E testing infrastructure, which is a valuable addition. However, there are several minor issues that should be addressed: **Issues to Fix:** 1. **Missing trailing newlines** in multiple files: - - - 2. **Package.json ordering** (): - should maintain alphabetical order in devDependencies 3. **Fragile test selectors** (): - Text-based assertions like and are brittle and language-dependent - Consider using data-testid attributes or more stable selectors **Recommendations:** - Add trailing newlines to maintain consistency - Use more robust E2E test selectors for better maintainability - Consider testing functionality rather than specific text content The Playwright configuration and CI integration look solid overall.

jank force-pushed playwright from 4c9bd97a49 to bcee6599b3 2025-06-04 09:59:14 +00:00 Compare

Claude commented 2025-06-04 10:00:38 +00:00

Collaborator

Copy link

Code Review for PR #283

Overall Assessment: Good addition of Playwright E2E testing infrastructure. The changes are mostly well-structured but need some minor fixes.

Issues to address:

1. frontend/playwright.config.ts:37 - Missing trailing semicolon at end of file
2. frontend/e2e/homepage.spec.ts:7-8 - Test assertions are fragile:
   • relies on exact German text
   • matches incomplete text that could change
     Consider using more robust selectors like attributes
3. frontend/.gitignore:32 - The entry appears unrelated to Playwright testing
4. frontend/package.json:42 - Unnecessary reordering of in devDependencies

Suggestions:

• Add attributes to key UI elements for more stable test selectors
• Consider if flag in CI is necessary (line 189 in ci.yml) as it may be resource-intensive

The Playwright setup and CI integration look solid overall. Please address the syntax error and consider improving test stability.

## Code Review for PR #283 **Overall Assessment:** Good addition of Playwright E2E testing infrastructure. The changes are mostly well-structured but need some minor fixes. **Issues to address:** 1. **frontend/playwright.config.ts:37** - Missing trailing semicolon at end of file 2. **frontend/e2e/homepage.spec.ts:7-8** - Test assertions are fragile: - relies on exact German text - matches incomplete text that could change Consider using more robust selectors like attributes 3. **frontend/.gitignore:32** - The entry appears unrelated to Playwright testing 4. **frontend/package.json:42** - Unnecessary reordering of in devDependencies **Suggestions:** - Add attributes to key UI elements for more stable test selectors - Consider if flag in CI is necessary (line 189 in ci.yml) as it may be resource-intensive The Playwright setup and CI integration look solid overall. Please address the syntax error and consider improving test stability.

jank force-pushed playwright from bcee6599b3 to 705482bab3 2025-06-04 10:01:26 +00:00 Compare

Claude commented 2025-06-04 10:02:35 +00:00

Collaborator

Copy link

Code Review

This PR successfully adds Playwright E2E testing with good CI integration. Here are my findings:

✅ Positive Points:

• Well-configured CI job with proper dependencies and triggers
• Standard Playwright configuration following best practices
• Appropriate package management and gitignore entries
• Good initial test coverage for homepage functionality

⚠️ Issues Found:

Critical:

• • Missing trailing newline (formatting violation)

Minor:

• • Consider more descriptive job name like 'playwright-e2e'
• • Unnecessary reordering of @angular/compiler-cli dependency

💡 Suggestions:

• Consider expanding the test in to cover more interactive elements
• The CI job installs many system packages (line 190) - consider a custom Docker image for faster builds

Overall, this is a solid foundation for E2E testing. Please fix the missing newline and consider the suggestions for improvement.

## Code Review This PR successfully adds Playwright E2E testing with good CI integration. Here are my findings: ### ✅ Positive Points: - Well-configured CI job with proper dependencies and triggers - Standard Playwright configuration following best practices - Appropriate package management and gitignore entries - Good initial test coverage for homepage functionality ### ⚠️ Issues Found: **Critical:** - - Missing trailing newline (formatting violation) **Minor:** - - Consider more descriptive job name like 'playwright-e2e' - - Unnecessary reordering of @angular/compiler-cli dependency ### 💡 Suggestions: - Consider expanding the test in to cover more interactive elements - The CI job installs many system packages (line 190) - consider a custom Docker image for faster builds Overall, this is a solid foundation for E2E testing. Please fix the missing newline and consider the suggestions for improvement.

jank force-pushed playwright from 705482bab3 to 52f706a75f 2025-06-04 10:02:39 +00:00 Compare

Claude commented 2025-06-04 10:04:25 +00:00

Collaborator

Copy link

Code Review - Issues Found

🔧 CI Workflow (.gitea/workflows/ci.yml:26-30)

• Line 26: The Reading package lists...
  Building dependency tree...
  Reading state information...
  0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. command should be formatted across multiple lines for better readability
• Consider pinning package versions for CI stability

📝 TypeScript Style (frontend/playwright.config.ts:37)

• Missing semicolon at end of file - inconsistent with project style

🧪 Test Quality (frontend/e2e/homepage.spec.ts:6-8)

• Lines 6-8: Hardcoded German text ('Willkommensbonus', 'von bis zu €') makes tests brittle
• Recommend using data attributes or more stable selectors instead

📦 Package Organization (frontend/package.json:42-44)

• devDependencies should be alphabetized (@angular/compiler-cli moved incorrectly)

✅ Positives:

• Great addition of E2E testing infrastructure
• Proper CI integration with conditional execution
• Good browser coverage in Playwright config

Recommendation: Fix the style issues and consider more robust test selectors before merging.

## Code Review - Issues Found ### 🔧 **CI Workflow (.gitea/workflows/ci.yml:26-30)** - Line 26: The Reading package lists... Building dependency tree... Reading state information... 0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. command should be formatted across multiple lines for better readability - Consider pinning package versions for CI stability ### 📝 **TypeScript Style (frontend/playwright.config.ts:37)** - Missing semicolon at end of file - inconsistent with project style ### 🧪 **Test Quality (frontend/e2e/homepage.spec.ts:6-8)** - Lines 6-8: Hardcoded German text ('Willkommensbonus', 'von bis zu €') makes tests brittle - Recommend using data attributes or more stable selectors instead ### 📦 **Package Organization (frontend/package.json:42-44)** - devDependencies should be alphabetized (@angular/compiler-cli moved incorrectly) ### ✅ **Positives:** - Great addition of E2E testing infrastructure - Proper CI integration with conditional execution - Good browser coverage in Playwright config **Recommendation:** Fix the style issues and consider more robust test selectors before merging.

jank force-pushed playwright from 52f706a75f to 7a2a60878e 2025-06-04 10:31:33 +00:00 Compare

jank force-pushed playwright from 7a2a60878e to db88a511b6 2025-06-04 10:36:28 +00:00 Compare

jank force-pushed playwright from db88a511b6 to 6a7b3cc550 2025-06-04 10:40:50 +00:00 Compare

jank force-pushed playwright from 6a7b3cc550 to ff1247d350 2025-06-04 10:44:39 +00:00 Compare

jank force-pushed playwright from ff1247d350 to 78d97489c1 2025-06-04 10:46:42 +00:00 Compare

jank force-pushed playwright from 78d97489c1 to 6984b82d28 2025-06-04 10:50:09 +00:00 Compare

jank force-pushed playwright from 6984b82d28 to ef610ab30d 2025-06-04 10:53:24 +00:00 Compare

jank force-pushed playwright from ef610ab30d to 6462111850 2025-06-04 10:59:31 +00:00 Compare

jank force-pushed playwright from 6462111850 to 21fb6656c5 2025-06-04 11:04:29 +00:00 Compare

jank force-pushed playwright from 21fb6656c5 to 3b29ce1797 2025-06-04 11:24:39 +00:00 Compare

jank force-pushed playwright from 3b29ce1797 to 81731ea14f 2025-06-04 11:46:44 +00:00 Compare

jank force-pushed playwright from 81731ea14f to aa3bd96881 2025-06-04 11:48:48 +00:00 Compare

jank force-pushed playwright from aa3bd96881 to b3b104329b 2025-06-04 11:53:47 +00:00 Compare

jank force-pushed playwright from b3b104329b to c87e1a9e2f 2025-06-04 11:56:12 +00:00 Compare

jank force-pushed playwright from c87e1a9e2f to 11e5436e67 2025-06-04 11:59:27 +00:00 Compare

jank force-pushed playwright from 11e5436e67 to 017d32bbb6 2025-06-04 12:03:40 +00:00 Compare

jank force-pushed playwright from 017d32bbb6 to 218bc611db 2025-06-04 12:12:56 +00:00 Compare

jank force-pushed playwright from 218bc611db to c9febc1b53 2025-06-04 12:17:49 +00:00 Compare

jank force-pushed playwright from c9febc1b53 to f50ff24b10 2025-06-04 12:28:26 +00:00 Compare

jank force-pushed playwright from f50ff24b10 to 9a1cf21b7c 2025-06-04 12:36:53 +00:00 Compare

jank force-pushed playwright from 9a1cf21b7c to 7f1faa39be 2025-06-04 12:48:58 +00:00 Compare

jank force-pushed playwright from 7f1faa39be to 325df279c2 2025-06-05 09:11:37 +00:00 Compare

jank force-pushed playwright from 325df279c2 to 5166fa6cc4 2025-06-05 09:31:41 +00:00 Compare

forgejo-actions added the

backend

label 2025-06-05 09:32:04 +00:00

jank force-pushed playwright from 5166fa6cc4 to f73084cdba 2025-06-05 09:58:50 +00:00 Compare

forgejo-actions commented 2025-06-05 09:59:07 +00:00

Copy link

👮‍♀️⚠️ This is a friendly reminder that the diff size of this PR is bigger than 200 lines we aim for. Please consider splitting this PR into more digestible pieces!

👮‍♀️⚠️ This is a friendly reminder that the diff size of this PR is bigger than 200 lines we aim for. Please consider splitting this PR into more digestible pieces!

forgejo-actions added

size

large

and removed

size

medium

labels 2025-06-05 09:59:07 +00:00

jank force-pushed playwright from f73084cdba to dfab3afd28 2025-06-05 10:11:17 +00:00 Compare

jank force-pushed playwright from dfab3afd28 to dbf7d0b458 2025-06-05 10:34:57 +00:00 Compare

jank force-pushed playwright from dbf7d0b458 to 4fafba5731 2025-06-05 10:43:27 +00:00 Compare

jank force-pushed playwright from 4fafba5731 to 071097a652 2025-06-05 10:45:53 +00:00 Compare

jank force-pushed playwright from 071097a652 to 138625f7f4 2025-06-05 10:51:13 +00:00 Compare

jank force-pushed playwright from 138625f7f4 to ae22692f80 2025-06-05 10:53:18 +00:00 Compare

jank force-pushed playwright from ae22692f80 to e828efdfa7 2025-06-05 11:16:58 +00:00 Compare

jank changed title from WIP: chore: Add some tests to chore: Add some tests 2025-06-05 11:20:56 +00:00

jank added 1 commit 2025-06-06 05:09:59 +00:00

fix: Rename variable back 111c6c2a64

csimonis commented 2025-06-06 11:58:43 +00:00

Owner

Copy link

fuck you

fuck you

csimonis approved these changes 2025-06-06 11:58:59 +00:00

jank merged commit 739c4f610a into main 2025-06-06 11:59:12 +00:00

jank deleted branch playwright 2025-06-06 11:59:12 +00:00

jank referenced this pull request from a commit 2025-06-06 11:59:13 +00:00

Merge pull request 'chore: Add some tests' (!283) from playwright into main

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

csimonis

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

  

backend

  

ci

  

docs

  

frontend

  

lgtm

blocked

  

lgtm

done

  

lgtm

need 1

  

size

huge

  

size

large

  

size

medium

  

size

small

No labels

Compat/Breaking

Kind/Bug

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

backend

ci

docs

frontend

lgtm

blocked

lgtm

done

lgtm

need 1

size

huge

size

large

size

medium

size

small

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

4 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: SZUT/casino#283

No description provided.