Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a multi-job GitHub Actions CI workflow; introduces Prettier config and ignore rules; updates package.json with formatting/testing scripts and dev tooling; adds Vitest configuration and test setup; and adds three new unit test suites. Also includes a new CLAUDE.md documentation file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as GitHub PR/Push
participant CI as GitHub Actions
participant Test as Job: test (matrix)
participant Format as Job: format-check
participant Build as Job: build (matrix)
participant Art as Coverage Artifact
participant PR as PR Comment
Dev->>CI: Trigger workflow
CI->>Test: Run on ubuntu/windows/macos (Node 18)
CI->>Format: Run Prettier check (ubuntu)
Test->>Test: npm ci • Prisma generate • lint (non-blocking)
Test->>Test: vitest --run
alt ubuntu only
Test->>Art: Upload coverage
Test->>PR: Post coverage summary
end
Test-->>CI: Completed (all matrix)
CI->>Build: Start after tests pass
Build->>Build: npm ci • Prisma generate • package (non-blocking errors)
Build-->>CI: Completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
app/.prettierrc (1)
1-11: Prettier config LGTM; consider normalizing EOL to avoid cross-OS diffs.With CI running across Linux/macOS/Windows,
"endOfLine": "auto"can cause churn when files are edited on Windows and checked on Linux. Suggest enforcing LF to make CI diffs stable. Also note that"singleQuote": falsewill prefer double quotes—ensure source files (e.g., test setup) use double quotes to satisfy format:check.Apply this diff if you want stable EOLs:
- "endOfLine": "auto" + "endOfLine": "lf"app/.prettierignore (1)
1-10: Ignore list is solid; add a few common files to reduce noise.Minor additions that commonly appear in repos (and aren’t useful to format): lockfiles for other managers, editor/system files, and Vitest artifacts.
Suggested diff:
node_modules/ .webpack/ dist/ coverage/ *.min.js *.min.css package-lock.json prisma/migrations/ .env *.log +pnpm-lock.yaml +yarn.lock +.DS_Store +.idea/ +.vscode/ +coverage-final.jsonapp/package.json (2)
15-20: Broaden Prettier globs to include config files used by CI/tests.Your scripts only target
src/**/*, so files likevitest.config.tsor top-level configs underapp/won’t be checked. Include them explicitly to catch drift early. Also, prefer the canonical--watchflag spelling.- "format": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\"", - "format:check": "prettier --check \"src/**/*.{ts,tsx,js,jsx,json,css,md}\"", + "format": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\" \"vitest.config.{ts,js}\"", + "format:check": "prettier --check \"src/**/*.{ts,tsx,js,jsx,json,css,md}\" \"vitest.config.{ts,js}\"", - "test:watch": "vitest watch", + "test:watch": "vitest --watch",
40-71: Align React versions, consolidate test environment, and modernize TypeScript
- In app/package.json (lines 88–89),
react-domis at^18.2.0whilereactis at^18.3.1; updatereact-domto^18.3.1to avoid mismatched peer issues.- Vitest config (
app/vitest.config.ts, lines 6–9) explicitly usesenvironment: 'jsdom'and setup files underjsdom; you can safely removehappy-dom(app/package.json line 58) to reduce dependency footprint.- The project’s TypeScript (
~4.5.4) is several major versions behind current tooling; consider bumping to the latest 5.x release for improved editor integrations and library definitions.Apply the minimal diffs:
--- a/app/package.json +++ b/app/package.json @@ dependencies/devDependencies - "react-dom": "^18.2.0", + "react-dom": "^18.3.1", - "happy-dom": "^12.10.3", - "typescript": "~4.5.4", + "typescript": "^5.0.0" // or the latest 5.x versionCLAUDE.md (4)
140-171: Fix markdownlint issues: remove trailing colons in headings.Headings with trailing colons trigger MD026 in many linters and editors. Drop the colon punctuation.
Example changes:
-### Workflow Jobs: +### Workflow Jobs -### Coverage Reporting: +### Coverage Reporting -### Files to Remove/Clean if Not Using ElectricSQL: +### Files to Remove/Clean if Not Using ElectricSQL -### Code to Clean in Existing Files: +### Code to Clean in Existing Files -### Current State: +### Current State
60-68: Make Python command examples cross-platform.Backslashes are Windows-specific. Offer POSIX variants or note OS-specific usage to prevent copy/paste errors on macOS/Linux.
-# Test camera fake stream (from root directory) -python pylon\pylon_stream_forever_fake.py "{\"num_frames\": 72, ...}" +# Test camera fake stream (from repo root) +# Windows: +python pylon\pylon_stream_forever_fake.py "{\"num_frames\": 72, ...}" +# macOS/Linux: +python pylon/pylon_stream_forever_fake.py '{"num_frames": 72, ...}'
129-131: Double-check publishing an internal IP in public docs.
10.0.0.23is RFC1918, but still reveals internal addressing. Confirm it’s safe to publish or replace with a placeholder like10.0.0.X.
1-174: Minor editorial polish (optional).
- Several list groups read as single paragraph due to missing blank lines before lists; adding a blank line improves Markdown rendering consistency.
- Static analysis flagged miscellaneous grammar nits; not blocking, but a quick pass with markdownlint and a grammar checker would clean this up.
- Consider noting in “Testing” that Vitest is configured with jsdom and that jest-dom is initialized via
app/src/test/setup.ts.app/src/renderer/PlantQrCodeTextBox.test.tsx (2)
23-31: Assert call count and prefer user-level events for realismConsider asserting the number of calls to avoid false positives and, optionally, using userEvent for closer-to-real typing semantics. fireEvent works, but userEvent is typically preferred for text input interactions.
Minimal improvement (no new deps required):
it('calls plantQrCodeChanged when input value changes', () => { const mockOnChange = vi.fn(); render(<PlantQrCodeTextBox qrCode="" plantQrCodeChanged={mockOnChange} />); const input = screen.getByRole('textbox'); fireEvent.change(input, { target: { value: 'NEW-QR-CODE' } }); - expect(mockOnChange).toHaveBeenCalledWith('NEW-QR-CODE'); + expect(mockOnChange).toHaveBeenCalledWith('NEW-QR-CODE'); + expect(mockOnChange).toHaveBeenCalledTimes(1); });If you already have @testing-library/user-event in devDependencies, you can further improve it:
-import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; -const input = screen.getByRole('textbox'); -fireEvent.change(input, { target: { value: 'NEW-QR-CODE' } }); +const input = screen.getByRole('textbox'); +await userEvent.type(input, 'NEW-QR-CODE');
33-44: Avoid assertions on a potentially stale DOM node after rerenderAfter rerender, it’s safer to re-query the element instead of reusing the previous reference. Also, this is a good place to assert that no onChange is fired by prop updates.
it('updates input value when qrCode prop changes', () => { const mockOnChange = vi.fn(); const { rerender } = render( <PlantQrCodeTextBox qrCode="OLD-CODE" plantQrCodeChanged={mockOnChange} /> ); - const input = screen.getByRole('textbox'); - expect(input).toHaveValue('OLD-CODE'); + expect(screen.getByRole('textbox')).toHaveValue('OLD-CODE'); rerender(<PlantQrCodeTextBox qrCode="NEW-CODE" plantQrCodeChanged={mockOnChange} />); - expect(input).toHaveValue('NEW-CODE'); + expect(screen.getByRole('textbox')).toHaveValue('NEW-CODE'); + expect(mockOnChange).not.toHaveBeenCalled(); });app/src/main/util.test.ts (2)
8-11: vi.resetModules is unnecessary here (static import + pure function)resolveHtmlPath reads process.env at call time and has no module-level side effects, so resetting the module registry in beforeEach doesn’t provide value. You can drop vi.resetModules for faster tests.
beforeEach(() => { - vi.resetModules(); process.env = { ...originalEnv }; });
13-15: Environment restore pattern is OK but can be saferAssigning the original object back is fine; alternatively, mirror the beforeEach cloning pattern to avoid object identity flips across tests. Optional:
afterEach(() => { - process.env = originalEnv; + process.env = { ...originalEnv }; });app/vitest.config.ts (1)
5-23: Run “main process” tests in a Node environment automaticallyYou currently set a global jsdom environment, which is perfect for renderer tests. For Electron “main” tests (e.g., app/src/main/**/*.test.ts), consider switching the environment to node using environmentMatchGlobs to avoid accidental DOM globals leaking into those tests.
export default defineConfig({ test: { globals: true, environment: 'jsdom', setupFiles: ['./src/test/setup.ts'], + // Ensure main-process tests run in a Node-like environment + environmentMatchGlobs: [ + ['src/main/**/*.test.ts', 'node'], + ], coverage: { provider: 'v8', reporter: ['text', 'json', 'html', 'lcov'],.github/workflows/ci.yml (1)
65-87: Format-check job should use the repo’s pinned Prettier and scriptInstead of installing Prettier ad hoc and running a bare glob, leverage your app/package.json script (format:check) to ensure consistent versions/config, and fail the job when formatting drifts (once you’re ready).
format-check: runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 18.x cache: 'npm' cache-dependency-path: app/package-lock.json - - - name: Install Prettier - working-directory: ./app - run: npm install --save-dev prettier - - - name: Check formatting - working-directory: ./app - run: npx prettier --check "src/**/*.{ts,tsx,js,jsx,json,css,md}" - continue-on-error: true # Don't fail for now since prettier isn't configured yet + - name: Install dependencies + working-directory: ./app + run: npm ci + + - name: Check formatting + working-directory: ./app + run: npm run format:check + continue-on-error: true # flip to 'false' once ready to gate on formatting
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.github/workflows/ci.yml(1 hunks)CLAUDE.md(1 hunks)app/.prettierignore(1 hunks)app/.prettierrc(1 hunks)app/package.json(2 hunks)app/src/main/util.test.ts(1 hunks)app/src/renderer/PlantQrCodeTextBox.test.tsx(1 hunks)app/src/renderer/util.test.ts(1 hunks)app/src/test/setup.ts(1 hunks)app/vitest.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/util.test.ts (1)
app/src/main/util.ts (1)
resolveHtmlPath(5-13)
app/src/renderer/PlantQrCodeTextBox.test.tsx (1)
app/src/renderer/PlantQrCodeTextBox.tsx (1)
PlantQrCodeTextBox(3-20)
app/src/renderer/util.test.ts (1)
app/src/renderer/util.ts (1)
getSupabaseClient(6-27)
🪛 GitHub Actions: CI
app/src/test/setup.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
app/src/main/util.test.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
app/src/renderer/PlantQrCodeTextBox.test.tsx
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
app/src/renderer/util.test.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
🪛 LanguageTool
CLAUDE.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...facing with scientific imaging hardware: - Camera Control: Basler Pylon camera in...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...lon camera integration for image capture - DAQ Integration: National Instruments ...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ardware control (stepper motor rotation) - Data Management: Local SQLite database...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...via Prisma ORM for storing scan metadata - Image Processing: Python-based image c...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... Development Commands ### Prerequisites 1. Activate conda environment: `mamba activ...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...quisites 1. Activate conda environment: mamba activate bloom-desktop 2. Ensure you're in the app directory for...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...chitecture ### Three-Layer Architecture 1. Electron Main Process (app/src/main/...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...lectron Main Process** (app/src/main/) - main.ts: Application entry point, window manage...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...plication entry point, window management - scanner.ts: Coordinates Python scripts for image c...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...dinates Python scripts for image capture - streamer.ts: Manages camera preview stream - `pr...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...eamer.ts: Manages camera preview stream - prismastore.ts: Database operations via Prisma - i...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...tore.ts: Database operations via Prisma - imageuploader.ts`: Handles image upload to storage - I...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...der.ts`: Handles image upload to storage - IPC handlers for renderer communication ...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...Renderer Process** (app/src/renderer/) - Component-based UI for scan capture, bro...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...for scan capture, browsing, and settings - Key components: CaptureScan.tsx, `Brow...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...x` - Uses React Router for navigation - Tailwind CSS for styling 3. **Python Ha...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...rdware Interface** (pylon/ and daq/) - pylon.py: Real camera control via pypylon - `...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...lon.py: Real camera control via pypylon - pylon_fake.py: Mock camera for testing - pylon_st...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...pylon_fake.py: Mock camera for testing - pylon_stream*.py: Various streaming implementations -...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...*.py: Various streaming implementations - test_daq.py, test_rot360.py`: DAQ control scripts ...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ..., test_rot360.py`: DAQ control scripts - Communication via spawn process and JSON...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ... and JSON arguments ### Database Schema - SQLite database managed by Prisma - Ke...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ... - SQLite database managed by Prisma - Key models: Phenotyper, Scientist, `...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...prisma/schema.prisma ### Configuration - Environment variables inapp/.env` (cre...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ... app/.env (create from .env.example) - Desktop config in `$HOME/.bloom/desktop-...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...eate from desktop-config.yaml.example) - Database URL configured via `BLOOM_DATAB...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...reload.ts) - All database operations go through main process ### Python Script Integra...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...n process ### Python Script Integration - Python scripts spawned as child processe...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ... spawned as child processes from Node.js - Arguments passed as JSON strings - Outpu...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...de.js - Arguments passed as JSON strings - Output captured via stdout/stderr stream...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...utput captured via stdout/stderr streams - Scripts return results as JSON or write ...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...ite files directly ### State Management - Local component state with React hooks -...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...- Local component state with React hooks - Database as source of truth for persiste...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...e as source of truth for persistent data - No global state management library (Redu...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...ibrary (Redux, etc.) ### Error Handling - Python scripts log to files in test dire...
(QB_NEW_EN)
[grammar] ~124-~124: There might be a mistake here.
Context: ... Handling - Python scripts log to files in test directory - Electron main process ...
(QB_NEW_EN)
[grammar] ~125-~125: There might be a mistake here.
Context: ... directory - Electron main process logs to console - Database errors handled in pr...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ...ed in prismastore.ts ## Important Notes - Camera IP address configured in Python s...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...ython scripts (10.0.0.23 for production) - Test images available in `test/sample_sc...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...ailable in test/sample_scan/ directory - Build tools required: Visual Studio C++ ...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...s), XCode (Mac), build-essential (Linux) - Always run npm run client:generate aft...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...re committing changes ## CI/CD Pipeline GitHub Actions workflow runs automatical...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ... Actions workflow runs automatically on: - Pull request creation/updates - Pushes t...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...ally on: - Pull request creation/updates - Pushes to main branch ### Workflow Jobs...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...ushes to main branch ### Workflow Jobs: 1. Test Job: Runs on Ubuntu, Windows, and...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...the application ### Coverage Reporting: - Coverage reports are generated using Vit...
(QB_NEW_EN)
[style] ~157-~157: Try moving the adverb to make the sentence clearer.
Context: ...at should be removed if the decision is made to permanently discontinue its use. ### Files to Remove/Clean if Not Usin...
(SPLIT_INFINITIVE)
[grammar] ~171-~171: There might be a mistake here.
Context: ...ic sync-related code ### Current State: - The app uses local SQLite database via P...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...ated code ### Current State: - The app uses local SQLite database via Prisma ORM - ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
140-140: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
151-151: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
159-159: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
165-165: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
171-171: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 115-115: trailing spaces
(trailing-spaces)
[error] 119-119: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
app/src/renderer/util.test.ts (1)
1-82: The script above will confirm whether Prettier is listed in your app’spackage.json. This is necessary because your formatting commands failed with “prettier: not found,” meaning the formatter isn’t installed locally.Once we see the output:
- If Prettier is missing, you’ll need to add it to
devDependencies(e.g.,npm install --save-dev prettier) and then re-runnpm run format.- If it’s present, we’ll investigate why the binary isn’t being resolved (perhaps update your
PATHor ensure a clean install).Let’s verify before proceeding.
.github/workflows/ci.yml (2)
42-55: Coverage comment action: point to coverage summary if you move reportsIf you later change coverage output locations, remember to update both upload-artifact and the vitest-coverage-report-action working-directory/vite-config-path. Current settings look consistent with app/vitest.config.ts.
1-119: CI workflow YAML formatting verified
All trailing whitespace has been removed and the file now ends with a newline. No further changes needed.
| it('returns localhost URL in development mode', () => { | ||
| process.env.NODE_ENV = 'development'; | ||
| process.env.PORT = '3000'; | ||
|
|
||
| const result = resolveHtmlPath('index.html'); | ||
|
|
||
| expect(result).toBe('http://localhost:3000/index.html'); | ||
| }); | ||
|
|
||
| it('uses default port 1212 when PORT is not set', () => { | ||
| process.env.NODE_ENV = 'development'; | ||
| delete process.env.PORT; | ||
|
|
||
| const result = resolveHtmlPath('index.html'); | ||
|
|
||
| expect(result).toBe('http://localhost:1212/index.html'); | ||
| }); | ||
|
|
||
| it('returns file URL in production mode', () => { | ||
| process.env.NODE_ENV = 'production'; | ||
|
|
||
| const result = resolveHtmlPath('index.html'); | ||
| const expectedPath = `file://${path.resolve(__dirname, '../renderer/index.html')}`; | ||
|
|
||
| expect(result).toBe(expectedPath); | ||
| }); | ||
|
|
||
| it('handles different HTML file names', () => { | ||
| process.env.NODE_ENV = 'development'; | ||
| process.env.PORT = '3000'; | ||
|
|
||
| const result = resolveHtmlPath('about.html'); | ||
|
|
||
| expect(result).toBe('http://localhost:3000/about.html'); | ||
| }); | ||
|
|
||
| it('returns file URL when NODE_ENV is not set', () => { | ||
| delete process.env.NODE_ENV; | ||
|
|
||
| const result = resolveHtmlPath('index.html'); | ||
| const expectedPath = `file://${path.resolve(__dirname, '../renderer/index.html')}`; | ||
|
|
||
| expect(result).toBe(expectedPath); | ||
| }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prettier warning: trailing whitespace
CI flagged a Prettier issue in this file as well. Likely trailing spaces on some blank lines. Please format:
🏁 Script executed:
#!/bin/bash
cd app
npm run format
npm run format:checkLength of output: 298
🏁 Script executed:
#!/bin/bash
set -e
cd app
echo "Searching for prettier entry in package.json"
grep -R '"prettier"' -n package.json || echo "No prettier entry found"
echo
echo "Searching for format script in package.json"
grep -R '"format"' -n package.json || echo "No format script found"Length of output: 404
🏁 Script executed:
#!/bin/bash
# Check for trailing whitespace in util.test.ts
grep -nE '[[:blank:]]+$' app/src/main/util.test.ts || echo "No trailing whitespace found"Length of output: 92
Remove trailing whitespace in util.test.ts
The file app/src/main/util.test.ts contains trailing spaces on several blank lines, which is flagged by Prettier. Please remove the trailing whitespace and re-format the file:
• File: app/src/main/util.test.ts
– Trailing spaces found at lines: 22, 31, 40, 49, 58
Suggested steps:
- Ensure dependencies are installed:
cd app npm install - Run the formatter via the project script:
or directly with npx:
npm run format
npx prettier --write "src/main/util.test.ts" - Verify no formatting issues remain:
npm run format:check
🤖 Prompt for AI Agents
In app/src/main/util.test.ts around lines 17 to 61 there are trailing spaces on
blank lines (lines 22, 31, 40, 49, 58); remove the trailing whitespace on those
lines (or run the project formatter) and reformat the file: run npm install in
the app folder if needed, then run npm run format or npx prettier --write
"src/main/util.test.ts", and finally verify formatting with npm run
format:check.
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { PlantQrCodeTextBox } from './PlantQrCodeTextBox'; | ||
|
|
||
| describe('PlantQrCodeTextBox', () => { | ||
| it('renders with empty value when qrCode is null', () => { | ||
| const mockOnChange = vi.fn(); | ||
| render(<PlantQrCodeTextBox qrCode={null} plantQrCodeChanged={mockOnChange} />); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| expect(input).toHaveValue(''); | ||
| }); | ||
|
|
||
| it('renders with provided qrCode value', () => { | ||
| const mockOnChange = vi.fn(); | ||
| const qrCode = 'PLANT-12345'; | ||
| render(<PlantQrCodeTextBox qrCode={qrCode} plantQrCodeChanged={mockOnChange} />); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| expect(input).toHaveValue('PLANT-12345'); | ||
| }); | ||
|
|
||
| it('calls plantQrCodeChanged when input value changes', () => { | ||
| const mockOnChange = vi.fn(); | ||
| render(<PlantQrCodeTextBox qrCode="" plantQrCodeChanged={mockOnChange} />); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| fireEvent.change(input, { target: { value: 'NEW-QR-CODE' } }); | ||
|
|
||
| expect(mockOnChange).toHaveBeenCalledWith('NEW-QR-CODE'); | ||
| }); | ||
|
|
||
| it('updates input value when qrCode prop changes', () => { | ||
| const mockOnChange = vi.fn(); | ||
| const { rerender } = render( | ||
| <PlantQrCodeTextBox qrCode="OLD-CODE" plantQrCodeChanged={mockOnChange} /> | ||
| ); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| expect(input).toHaveValue('OLD-CODE'); | ||
|
|
||
| rerender(<PlantQrCodeTextBox qrCode="NEW-CODE" plantQrCodeChanged={mockOnChange} />); | ||
| expect(input).toHaveValue('NEW-CODE'); | ||
| }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prettier warning: trailing whitespace
CI flagged a Prettier formatting issue. A few blank lines contain trailing spaces. Please run the formatter in the app workspace to normalize:
🏁 Script executed:
#!/bin/bash
cd app
npm run format
npm run format:checkLength of output: 298
🏁 Script executed:
#!/bin/bash
# Search for trailing whitespace in the test file
rg -nP '[ \t]+$' -n app/src/renderer/PlantQrCodeTextBox.test.tsxLength of output: 113
Remove trailing whitespace in PlantQrCodeTextBox.test.tsx
Please remove the trailing spaces on the blank lines identified below. These extra spaces can cause CI/prettier checks to fail once Prettier is available in the workspace.
Files and lines to fix:
- app/src/renderer/PlantQrCodeTextBox.test.tsx:
• Line 9
• Line 18
• Line 26
• Line 29
• Line 38
• Line 41
You can strip these manually or configure/install Prettier in the app workspace and re-run your formatting scripts to automatically clean up the trailing whitespace.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
🤖 Prompt for AI Agents
In app/src/renderer/PlantQrCodeTextBox.test.tsx around lines 1 to 45: there are
trailing whitespace characters on the blank lines at lines 9, 18, 26, 29, 38 and
41; remove the trailing spaces from those lines (or run Prettier/your formatter
in the app workspace) so the file has no trailing whitespace and formatting/CI
checks will pass.
| vi.stubGlobal('window', { | ||
| electron: { | ||
| bloom: { | ||
| getCredentials: mockGetCredentials, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Don’t clobber the jsdom window object
vi.stubGlobal('window', { … }) replaces the entire window object, potentially breaking jsdom APIs for this test file. Patch only window.electron instead.
-vi.stubGlobal('window', {
- electron: {
- bloom: {
- getCredentials: mockGetCredentials,
- },
- },
-});
+Object.defineProperty(window, 'electron', {
+ value: {
+ bloom: {
+ getCredentials: mockGetCredentials,
+ },
+ },
+ configurable: true,
+});Optionally, add an afterAll to clean up:
afterAll(() => {
// @ts-expect-error test-only cleanup
delete (window as any).electron;
});🤖 Prompt for AI Agents
In app/src/renderer/util.test.ts around lines 6 to 12, the test currently calls
vi.stubGlobal('window', {...}) which replaces the entire jsdom window object;
instead, assign or define only the window.electron property with the same nested
bloom.getCredentials mock so you don't clobber jsdom APIs, and add an afterAll
cleanup that deletes window.electron (use a ts-expect-error comment if needed
for the delete) to restore the environment after the test.
| vi.mock('@supabase/supabase-js', () => ({ | ||
| createClient: (...args: any[]) => { | ||
| const client = mockCreateClient(...args); | ||
| return { | ||
| ...client, | ||
| auth: { | ||
| signInWithPassword: mockSignInWithPassword, | ||
| }, | ||
| }; | ||
| }, | ||
| })); |
There was a problem hiding this comment.
Mock ordering bug: supabase mock must be defined before importing the module
getSupabaseClient’s module imports createClient at top-level. vi.mock must be declared before importing ./util; otherwise, the real module is used. This can cause real network/config usage.
-// Now import the module after mocking
-const { getSupabaseClient } = await import('./util');
-
-const mockCreateClient = vi.fn();
-const mockSignInWithPassword = vi.fn();
-
-vi.mock('@supabase/supabase-js', () => ({
+const mockCreateClient = vi.fn();
+const mockSignInWithPassword = vi.fn();
+
+vi.mock('@supabase/supabase-js', () => ({
createClient: (...args: any[]) => {
const client = mockCreateClient(...args);
return {
...client,
auth: {
signInWithPassword: mockSignInWithPassword,
},
};
},
}));
+
+// Now import the module after mocking
+const { getSupabaseClient } = await import('./util');📝 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.
| vi.mock('@supabase/supabase-js', () => ({ | |
| createClient: (...args: any[]) => { | |
| const client = mockCreateClient(...args); | |
| return { | |
| ...client, | |
| auth: { | |
| signInWithPassword: mockSignInWithPassword, | |
| }, | |
| }; | |
| }, | |
| })); | |
| const mockCreateClient = vi.fn(); | |
| const mockSignInWithPassword = vi.fn(); | |
| vi.mock('@supabase/supabase-js', () => ({ | |
| createClient: (...args: any[]) => { | |
| const client = mockCreateClient(...args); | |
| return { | |
| ...client, | |
| auth: { | |
| signInWithPassword: mockSignInWithPassword, | |
| }, | |
| }; | |
| }, | |
| })); | |
| // Now import the module after mocking | |
| const { getSupabaseClient } = await import('./util'); |
🤖 Prompt for AI Agents
In app/src/renderer/util.test.ts around lines 20 to 30, the supabase mock is
declared after the module under test is imported which causes the real
createClient to be used; move the vi.mock('@supabase/supabase-js', ...)
declaration to run before importing ./util (or call vi.resetModules() then
declare the mock and dynamically import the module under test), then
import/getSupabaseClient so the mocked createClient is used; optionally
restore/reset mocks after the test.
| it('handles authentication failure', async () => { | ||
| const mockCredentials = { | ||
| bloom_api_url: 'https://test.supabase.co', | ||
| bloom_anon_key: 'test-anon-key', | ||
| email: 'test@example.com', | ||
| password: 'wrong-password', | ||
| }; | ||
|
|
||
| mockGetCredentials.mockResolvedValue(mockCredentials); | ||
| mockCreateClient.mockReturnValue({}); | ||
| mockSignInWithPassword.mockRejectedValue(new Error('Invalid credentials')); | ||
|
|
||
| await expect(getSupabaseClient()).rejects.toThrow('Invalid credentials'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Supabase signInWithPassword typically resolves with an error instead of rejecting
The library returns a resolved object with an error field on failure; it doesn’t reject the promise. Your test currently uses mockRejectedValue, which misrepresents the real behavior and could mask production issues.
Align the mock with Supabase semantics and (ideally) update the implementation to check the returned error.
Test fix:
it('handles authentication failure', async () => {
const mockCredentials = {
bloom_api_url: 'https://test.supabase.co',
bloom_anon_key: 'test-anon-key',
email: 'test@example.com',
password: 'wrong-password',
};
mockGetCredentials.mockResolvedValue(mockCredentials);
mockCreateClient.mockReturnValue({});
- mockSignInWithPassword.mockRejectedValue(new Error('Invalid credentials'));
+ mockSignInWithPassword.mockResolvedValue({
+ data: null,
+ error: new Error('Invalid credentials'),
+ });
- await expect(getSupabaseClient()).rejects.toThrow('Invalid credentials');
+ await expect(getSupabaseClient()).rejects.toThrow('Invalid credentials');
});Production code improvement (outside this file, in app/src/renderer/util.ts):
// Inside getSupabaseClient after signInWithPassword call
const { error } = await supabase.auth.signInWithPassword({
email: credentials.email,
password: credentials.password,
});
if (error) {
throw error;
}I can push a small PR update with these changes if you’d like.
🤖 Prompt for AI Agents
In app/src/renderer/util.test.ts around lines 68 to 81, the test incorrectly
uses mockRejectedValue for supabase.auth.signInWithPassword which misrepresents
Supabase behavior; change the mock to mockResolvedValue({ data: null, error: new
Error('Invalid credentials') }) so the test mirrors the real resolved-with-error
response, and update the production getSupabaseClient implementation
(app/src/renderer/util.ts) to inspect the returned { error } from
signInWithPassword and throw the error if present (e.g. const { error } = await
supabase.auth.signInWithPassword(...); if (error) throw error).
| import '@testing-library/jest-dom'; | ||
| import { cleanup } from '@testing-library/react'; | ||
| import { afterEach } from 'vitest'; | ||
|
|
||
| afterEach(() => { | ||
| cleanup(); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
Fix Prettier failure and use Vitest-specific jest-dom entry.
Pipeline flagged a Prettier issue; your Prettier config prefers double quotes. Also, when using Vitest, prefer importing @testing-library/jest-dom/vitest so matchers are added to Vitest’s expect.
Apply this diff:
-import '@testing-library/jest-dom';
-import { cleanup } from '@testing-library/react';
-import { afterEach } from 'vitest';
+import "@testing-library/jest-dom/vitest";
+import { cleanup } from "@testing-library/react";
+import { afterEach } from "vitest";
-afterEach(() => {
- cleanup();
-});
+afterEach(() => {
+ cleanup();
+});📝 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.
| import '@testing-library/jest-dom'; | |
| import { cleanup } from '@testing-library/react'; | |
| import { afterEach } from 'vitest'; | |
| afterEach(() => { | |
| cleanup(); | |
| }); | |
| import "@testing-library/jest-dom/vitest"; | |
| import { cleanup } from "@testing-library/react"; | |
| import { afterEach } from "vitest"; | |
| afterEach(() => { | |
| cleanup(); | |
| }); |
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix code style issues in this file.
🤖 Prompt for AI Agents
In app/src/test/setup.ts around lines 1 to 7, Prettier prefers double quotes and
you should use the Vitest-specific jest-dom entry; change the import strings
from single to double quotes and replace "@testing-library/jest-dom" with
"@testing-library/jest-dom/vitest" (keep the cleanup import and afterEach call
intact), ensuring all import literals use double quotes so the file conforms to
Prettier and Vitest matcher setup.
Coverage Report for ./app
File Coverage
|
||||||||||||||||||||||||||||||||||||||
eberrigan
force-pushed
the
elizabeth/add-testing
branch
from
623594c to
9e3627a
Compare
|
Add something like the following to the workflow and assume |
Summary by CodeRabbit