Skip to content

Add end-to-end testing for fdm-app for critical user journeys#210

Closed
BoraIneviNMI wants to merge 15 commits into
developmentfrom
feature/e2e-testing
Closed

Add end-to-end testing for fdm-app for critical user journeys#210
BoraIneviNMI wants to merge 15 commits into
developmentfrom
feature/e2e-testing

Conversation

@BoraIneviNMI
Copy link
Copy Markdown
Collaborator

@BoraIneviNMI BoraIneviNMI commented Jul 30, 2025

  • New Features
    • To ensure the long-term stability and quality of fdm-app, we will implement an end-to-end (E2E) testing suite using Playwright. The primary goals are to guarantee that critical user journeys remain functional and to prevent unintended visual regressions in the UI.
    • These tests will run automatically on all Pull Requests against main and development, acting as a quality gate before merging.
    • Tests are added in the fdm-app/tests directory. Since they are not unit tests they aren't added next to the source files unlike in other packages.
    • There will be a test file for each action the user can take, and one for each step of the farm creation wizard.
  • Improvements
    • Add end to end testing for e-mail magic link and other forms of authentication

Summary by CodeRabbit

  • New Features

    • Introduced end-to-end testing with Playwright, including browser-based sign-in test automation.
    • Added Playwright configuration for multi-browser testing and HTML reporting.
    • Integrated code coverage reporting for tests.
  • Chores

    • Updated GitHub Actions workflow to run tests and upload coverage for the app.
    • Enhanced .gitignore to exclude additional build, test, and coverage artifacts.
    • Added code coverage configuration to exclude irrelevant files and directories.
  • Tests

    • Added a Playwright test for sign-in via magic link.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jul 30, 2025

⚠️ No Changeset found

Latest commit: 0104a68

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 30, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

"""

Walkthrough

A Playwright-based end-to-end (E2E) testing setup has been introduced for the fdm-app component. This includes configuration files, test scripts, a sample test, code coverage integration, and updates to the CI workflow to run these tests with coverage reporting. Supporting dependencies and .gitignore rules were also updated.

Changes

Cohort / File(s) Change Summary
CI Workflow: Playwright Integration
.github/workflows/tests.yml
Added a test-app job to run Playwright E2E tests for fdm-app in CI, including PostgreSQL service, coverage collection, and Codecov upload.
Playwright Test Configuration
fdm-app/playwright.config.ts
Introduced Playwright configuration: test directory, browser projects, CI-specific settings, HTML reporting, and web server startup for tests.
Sample Playwright Test
fdm-app/tests/signin._index.test.ts
Added a test automating sign-in via magic link, including cookie banner handling and UI assertions.
Coverage Configuration
fdm-app/.c8rc
Added .c8rc to configure code coverage reporting and exclusions for E2E tests.
Git Ignore Rules
fdm-app/.gitignore
Expanded ignore list for build, test, and coverage artifacts.
Package and Scripts Update
fdm-app/package.json
Added Playwright and c8 as devDependencies, and a test script to run Playwright tests with environment variables.

Sequence Diagram(s)

sequenceDiagram
    participant CI as GitHub Actions (test-app job)
    participant Docker as Node.js 22 Container
    participant Postgres as PostgreSQL Service
    participant App as fdm-app Web Server
    participant Playwright as Playwright Test Runner

    CI->>Docker: Start container & install dependencies
    Docker->>Postgres: Start PostgreSQL service
    Docker->>App: Build and start fdm-app server (with coverage)
    Docker->>Playwright: Run Playwright E2E tests
    Playwright->>App: Execute browser-based tests (e.g., sign-in)
    Playwright->>Docker: Generate coverage reports
    Docker->>CI: Upload coverage to Codecov
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15–20 minutes

Possibly related issues

Poem

In the warren where code and tests hop,
Playwright arrives with a click and a pop!
E2E bunnies now bound through the app,
Checking sign-ins with a whisker tap.
With coverage in tow and CI on the run,
This rabbit declares: "Testing is fun!" 🐇✨
"""

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/e2e-testing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot added the fdm-app label Jul 30, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
fdm-app/package.json (1)

85-86: Remove duplicate Playwright packages

You usually need either @playwright/test (includes browsers) or playwright, not both. Keeping both risks version drift.

Drop playwright unless you have a specific reason.

Also applies to: 101-103

fdm-app/.gitignore (1)

9-14: Keep ignore-pattern style consistent (add leading / to new directories)

Earlier entries such as /build and /.cache use a leading slash to scope the ignore rule to the package root. The newly-added patterns omit the slash, which causes them to match dist, coverage, etc. at any depth in fdm-app/. That’s harmless but inconsistent and may hide similarly-named directories inside src or test fixtures.

Diff to align with existing style:

- dist
- dist-ssr
- playwright-report
- test-results
- coverage
+ /dist
+ /dist-ssr
+ /playwright-report
+ /test-results
+ /coverage

(Optional) You could also add a trailing / to signal they are directories, but that’s stylistic.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2716a8c and 73ddbdd.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • .github/workflows/tests.yml (1 hunks)
  • fdm-app/.c8rc (1 hunks)
  • fdm-app/.gitignore (1 hunks)
  • fdm-app/package.json (3 hunks)
  • fdm-app/playwright.config.ts (1 hunks)
  • fdm-app/tests/signin._index.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Learnt from: SvenVw
PR: SvenVw/fdm#65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.
Learnt from: SvenVw
PR: SvenVw/fdm#6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Learnt from: SvenVw
PR: SvenVw/fdm#9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Learnt from: SvenVw
PR: SvenVw/fdm#20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the `fdm-app/tsconfig.json` file, the include path `.react-router/types/**/*` refers to a build-time generated directory which is intentionally not included in the repository.
Learnt from: SvenVw
PR: SvenVw/fdm#163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
fdm-app/.gitignore (3)

Learnt from: SvenVw
PR: #20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the fdm-app/tsconfig.json file, the include path .react-router/types/**/* refers to a build-time generated directory which is intentionally not included in the repository.

Learnt from: SvenVw
PR: #6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the fdm-app project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in vite.config.ts.

Learnt from: SvenVw
PR: #65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.

fdm-app/package.json (7)

Learnt from: SvenVw
PR: #6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the fdm-app project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in vite.config.ts.

Learnt from: SvenVw
PR: #95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's vi mocking utilities and prefer manual JavaScript mocks.

Learnt from: SvenVw
PR: #9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In fdm-data/src/cultivations/index.test.ts, the fdm object created by drizzle does not have an .end() method. Cleanup code should not attempt to call fdm.end();.

Learnt from: SvenVw
PR: #20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the fdm-app/tsconfig.json file, the include path .react-router/types/**/* refers to a build-time generated directory which is intentionally not included in the repository.

Learnt from: SvenVw
PR: #147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax "typescript": "catalog:" is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo.

Learnt from: SvenVw
PR: #124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.

Learnt from: SvenVw
PR: #147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax "typescript": "catalog:" is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo. The catalog: syntax without a package name is the shorthand for referencing the default catalog.

fdm-app/.c8rc (2)

Learnt from: SvenVw
PR: #6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the fdm-app project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in vite.config.ts.

Learnt from: SvenVw
PR: #20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the fdm-app/tsconfig.json file, the include path .react-router/types/**/* refers to a build-time generated directory which is intentionally not included in the repository.

fdm-app/tests/signin._index.test.ts (2)

Learnt from: SvenVw
PR: #151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.

Learnt from: SvenVw
PR: #23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.

fdm-app/playwright.config.ts (2)

Learnt from: SvenVw
PR: #6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the fdm-app project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in vite.config.ts.

Learnt from: SvenVw
PR: #65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.

.github/workflows/tests.yml (1)

Learnt from: SvenVw
PR: #65
File: .github/workflows/check.yml:67-76
Timestamp: 2025-01-29T14:32:14.684Z
Learning: For temporary test databases that are created and destroyed within GitHub Actions workflows and run in isolated containers, using default credentials (like postgres/postgres) is acceptable as they pose no security risk.

🔇 Additional comments (3)
.github/workflows/tests.yml (1)

326-346: Coverage upload path may be empty – CI will error out.

The step runs pnpm test but the upload expects ./fdm-app/coverage/tmp/*.json.
Confirm that the test script invokes c8/nyc with --report-dir coverage/tmp; otherwise the Codecov step will fail with “no matching files”.
Consider renaming the step to Run tests with coverage and using the same pnpm test-coverage convention as the other jobs for consistency.

fdm-app/package.json (1)

13-14: test script won’t start a built server

playwright.config.ts launches react-router-serve ./build/..., but pnpm run test never runs pnpm build first, so the server file may be missing locally.

Consider:

-"test": "dotenvx run playwright test",
+"test": "pnpm build && dotenvx run playwright test",

or point webServer.command at react-router dev in non-CI runs.

fdm-app/playwright.config.ts (1)

45-52: Ensure build exists before server start

react-router-serve ./build/server/index.js will fail if ./build hasn’t been produced. Confirm your local/CI workflow builds the app before Playwright starts, or switch to react-router dev for local runs.

Comment thread .github/workflows/tests.yml
Comment thread .github/workflows/tests.yml
Comment thread fdm-app/.c8rc Outdated
Comment on lines +8 to +9
"**.d.ts",
"*.config.ts",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect glob pattern

"**.d.ts" should be "**/*.d.ts" to actually exclude declaration files.
Same applies if you intend "**/*.d.ts?(x)" for both js/ts declarations.

🤖 Prompt for AI Agents
In fdm-app/.c8rc around lines 8 to 9, the glob pattern "**.d.ts" is incorrect
and does not properly match declaration files. Change it to "**/*.d.ts" to
correctly exclude all declaration files in any subdirectory. If you want to
exclude both .d.ts and .d.tsx files, use the pattern "**/*.d.ts?(x)" instead.

Comment thread fdm-app/tests/signin._index.test.ts Outdated
Comment thread fdm-app/tests/signin._index.test.ts Outdated
Comment on lines +26 to +28
const message = page.getByText("aanmelden...")
expect(message).toBeDefined()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Assertion does not verify visibility

expect(message).toBeDefined() always passes because the locator is always defined. Use:

- expect(message).toBeDefined()
+ await expect(message).toBeVisible()
📝 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
const message = page.getByText("aanmelden...")
expect(message).toBeDefined()
const message = page.getByText("aanmelden...")
- expect(message).toBeDefined()
+ await expect(message).toBeVisible()
🤖 Prompt for AI Agents
In fdm-app/tests/signin._index.test.ts around lines 26 to 28, the assertion uses
expect(message).toBeDefined(), which only checks if the locator exists but does
not verify if the element is visible on the page. Replace this with an assertion
that checks the visibility of the element, such as
expect(message).toBeVisible(), to ensure the element is actually displayed to
the user.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.07%. Comparing base (19a512d) to head (0104a68).
⚠️ Report is 1286 commits behind head on development.

Additional details and impacted files
@@             Coverage Diff              @@
##           development     #210   +/-   ##
============================================
  Coverage        93.07%   93.07%           
============================================
  Files               81       81           
  Lines            13118    13118           
  Branches          1326     1326           
============================================
  Hits             12209    12209           
  Misses             907      907           
  Partials             2        2           
Flag Coverage Δ
fdm-calculator 94.65% <ø> (ø)
fdm-core 91.92% <ø> (ø)
fdm-data 94.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)

318-321: Add a cache for Playwright browser binaries to cut ~50-60 s off every run.

pnpm exec playwright install --with-deps downloads ~100 MB of browsers every job.
The binaries are architecture-stable and can be cached safely between runs.

+      - name: Cache Playwright browsers
+        uses: actions/cache@v4
+        with:
+          path: ~/.cache/ms-playwright
+          key: ${{ runner.os }}-playwright-${{ hashFiles('fdm-app/pnpm-lock.yaml', 'pnpm-lock.yaml') }}
+          restore-keys: |
+            ${{ runner.os }}-playwright-
+
       - name: Install Playwright browsers
-        run: pnpm exec playwright install --with-deps
+        run: pnpm exec playwright install --with-deps
         working-directory: ./fdm-app
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 272b85f and cef0983.

📒 Files selected for processing (1)
  • .github/workflows/tests.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Learnt from: SvenVw
PR: SvenVw/fdm#65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Learnt from: SvenVw
PR: SvenVw/fdm#6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Learnt from: SvenVw
PR: SvenVw/fdm#9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Learnt from: SvenVw
PR: SvenVw/fdm#20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the `fdm-app/tsconfig.json` file, the include path `.react-router/types/**/*` refers to a build-time generated directory which is intentionally not included in the repository.
Learnt from: SvenVw
PR: SvenVw/fdm#163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Learnt from: SvenVw
PR: SvenVw/fdm#95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
.github/workflows/tests.yml (11)

Learnt from: SvenVw
PR: #65
File: .github/workflows/check.yml:67-76
Timestamp: 2025-01-29T14:32:14.684Z
Learning: For temporary test databases that are created and destroyed within GitHub Actions workflows and run in isolated containers, using default credentials (like postgres/postgres) is acceptable as they pose no security risk.

Learnt from: SvenVw
PR: #6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the fdm-app project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in vite.config.ts.

Learnt from: SvenVw
PR: #9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In fdm-data/src/cultivations/index.test.ts, the fdm object created by drizzle does not have an .end() method. Cleanup code should not attempt to call fdm.end();.

Learnt from: SvenVw
PR: #95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's vi mocking utilities and prefer manual JavaScript mocks.

Learnt from: SvenVw
PR: #20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the fdm-app/tsconfig.json file, the include path .react-router/types/**/* refers to a build-time generated directory which is intentionally not included in the repository.

Learnt from: SvenVw
PR: #45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the redirect function should be imported from react-router, not react-router-dom.

Learnt from: SvenVw
PR: #9
File: fdm-core/src/cultivation.d.ts:3-13
Timestamp: 2024-11-27T09:58:24.047Z
Learning: In the fdm-core TypeScript codebase, interface names should not be prefixed with 'I'; interface names should be meaningful without the 'I' prefix.

Learnt from: SvenVw
PR: #71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.

Learnt from: SvenVw
PR: #65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.

Learnt from: SvenVw
PR: #45
File: fdm-app/app/routes/farm.$b_id_farm.settings._index.tsx:1-1
Timestamp: 2025-01-14T16:06:21.832Z
Learning: In the fdm project, redirect and other routing utilities should be imported from react-router instead of react-router-dom.

Learnt from: SvenVw
PR: #95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.

🔇 Additional comments (1)
.github/workflows/tests.yml (1)

334-336: Possible duplicate compile – reconsider the explicit pnpm build fdm-app.

If playwright.config.ts starts the dev server via the webServer option (common pattern), a prior pnpm build is unnecessary and just costs CI time.
Consider removing this step or comment why a production build is required here.

Comment thread .github/workflows/tests.yml Outdated
Comment on lines +342 to +364
# The hostname used to communicate with the PostgreSQL service container
POSTGRES_HOST: postgres
# The default PostgreSQL port
POSTGRES_PORT: 5432
# the default usernam
POSTGRES_USER: postgres
# the default password
POSTGRES_PASSWORD: postgres
# the default database
POSTGRES_DB: postgres

# Public name of the application displayed in the UI.
PUBLIC_FDM_NAME: MINAS4
# Base URL of the application (used for API calls, etc.).
PUBLIC_FDM_URL: http://localhost:3000
# URL to the privacy policy document.
PUBLIC_FDM_PRIVACY_URL: http://localhost:3000/privacy
# Secret key used to sign session cookies. MUST be a strong, random string.
FDM_SESSION_SECRET: blablabla1
# Authentication (Better Auth & OAuth Providers)
BETTER_AUTH_SECRET: blablabla2
# Full base URL of this application (used for redirects by better-auth).
BETTER_AUTH_URL: http://localhost:3000
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Move hard-coded secrets to repository/organization secrets.

FDM_SESSION_SECRET, BETTER_AUTH_SECRET and other secrets are embedded in the workflow file.
Even though the runner is ephemeral, keeping real-looking secrets in plain text is a bad habit and may be copied into other contexts.

Store them in Secrets → Actions and reference with ${{ secrets.FDM_SESSION_SECRET }} etc.

🤖 Prompt for AI Agents
In .github/workflows/tests.yml around lines 342 to 364, the secrets like
FDM_SESSION_SECRET and BETTER_AUTH_SECRET are hard-coded in plain text, which is
insecure. Move these secret values to the repository or organization secrets
under GitHub Actions Secrets, then replace the hard-coded values in the workflow
file with references to these secrets using the syntax ${{ secrets.SECRET_NAME
}} to securely inject them during runtime.

Comment on lines +370 to +375
- name: fdm-app - Upload results to Codecov
uses: codecov/codecov-action@v5
with:
files: ./fdm-app/coverage/tmp/*.json
flags: fdm-app
token: ${{ secrets.CODECOV_TOKEN }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Codecov step might miss files when the glob expands to multiple paths.

files: ./fdm-app/coverage/tmp/*.json expands to a space-separated list which Codecov v5 does not parse.
Either:

  1. Merge coverage first (npx playwright merge-coverage → single coverage-final.json)
  2. Pass a newline-separated list via the multiline YAML literal.

Example fix:

-          files: ./fdm-app/coverage/tmp/*.json
+          files: |
+            ./fdm-app/coverage/tmp/**/coverage*.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
- name: fdm-app - Upload results to Codecov
uses: codecov/codecov-action@v5
with:
files: ./fdm-app/coverage/tmp/*.json
flags: fdm-app
token: ${{ secrets.CODECOV_TOKEN }}
- name: fdm-app - Upload results to Codecov
uses: codecov/codecov-action@v5
with:
files: |
./fdm-app/coverage/tmp/**/coverage*.json
flags: fdm-app
token: ${{ secrets.CODECOV_TOKEN }}
🤖 Prompt for AI Agents
In .github/workflows/tests.yml around lines 370 to 375, the Codecov upload step
uses a glob pattern that expands to a space-separated list of files, which
Codecov v5 does not handle correctly. To fix this, either merge the coverage
files into a single file before uploading or change the files input to a
newline-separated list using YAML multiline literal syntax to ensure Codecov
correctly processes all coverage files.

@SvenVw SvenVw marked this pull request as draft July 31, 2025 11:47
@BoraIneviNMI BoraIneviNMI linked an issue Aug 5, 2025 that may be closed by this pull request
@BoraIneviNMI BoraIneviNMI self-assigned this Aug 7, 2025
@SvenVw SvenVw closed this Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement E2E Testing for Critical User Journeys

2 participants