Skip to content

Fix failing react tests#318

Closed
rnarciso wants to merge 25 commits intocoleam00:mainfrom
rnarciso:fix-failing-react-tests
Closed

Fix failing react tests#318
rnarciso wants to merge 25 commits intocoleam00:mainfrom
rnarciso:fix-failing-react-tests

Conversation

@rnarciso
Copy link
Copy Markdown

Pull Request

Summary

Changes Made

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

# Example: python -m pytest tests/
# Example: cd archon-ui-main && npm run test

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

Additional Notes

google-labs-jules Bot and others added 25 commits August 15, 2025 15:25
The frontend was failing to communicate with the backend in development because it was constructing an absolute URL for API requests, bypassing the Vite proxy.

This change modifies the `getApiUrl` function in `src/config/api.ts` to return a relative path ('') in development mode. This ensures all API and WebSocket requests are correctly proxied by the Vite dev server, resolving the connection and CORS issues.
The React UI tests were failing in environments with older Node.js versions due to the use of `crypto.randomUUID()`, which is not universally available.

This commit replaces the call to `crypto.randomUUID()` in `src/services/testService.ts` with `uuidv4()` from the `uuid` library. The `uuid` library and its types have been added to the project dependencies.

This ensures compatibility and allows the UI tests to run correctly across different development environments.
This commit addresses three issues:
1. A UI crash when clicking the "Dashboard" button without any test results. This was fixed by adding a null check for the test results summary.
2. The React UI tests not running. This was fixed by changing the test command in the vite config to use a non-interactive reporter and fixing the ANSI escape code stripping.
3. A layout issue in the dashboard where the "Test Summary" and "Coverage Analysis" boxes were displayed side-by-side. This was fixed by changing the grid layout to stack them vertically.
This commit addresses three issues:
1. A UI crash when clicking the "Dashboard" button without any test results. This was fixed by adding robust null checks, optional chaining, and default values for the test results summary and suites.
2. The React UI tests not running. This was fixed by changing the test command in the vite config to use the `dot` reporter, which is more reliable for streaming output.
3. A layout issue in the dashboard where the "Test Summary" and "Coverage Analysis" boxes were displayed side-by-side. This was fixed by changing the grid layout to stack them vertically.
The React UI tests were failing with the error "AssertionError: expected false to be true". This was caused by a bug in the `isLmConfigured` function in `archon-ui-main/src/utils/onboarding.ts`.

The `hasValidCredential` helper function was not correctly handling encrypted credentials. It was checking for the presence of `cred.encrypted_value`, but it should have been checking if `cred.is_encrypted` is `true`.

This change updates the `hasValidCredential` function to correctly handle encrypted credentials.
A test in `test/errors.test.tsx` was failing with the error `TestingLibraryElementError: Unable to find an accessible element with the role "alert"`.

This was caused by a flawed test that was not correctly handling asynchronous operations. The test was using `setTimeout` to simulate a delay, but it was not waiting for the delayed code to execute before trying to find the "alert" element.

This change fixes the test by using `vitest`'s fake timers (`vi.useFakeTimers()` and `vi.advanceTimersByTimeAsync()`) and `async/await` with `screen.findByRole('alert')`. This makes the test more robust and reliable.
A test in `test/errors.test.tsx` was failing with the error `ReferenceError: act is not defined`.

This was caused by a missing import for the `act` function from `@testing-library/react`. The `act` function was used in the `timeout error simulation` test, but it was not imported.

This change adds `act` to the import statement at the top of the file to resolve the error.
The `npm test` script was not generating coverage reports, which caused the test summary dashboard to show all zeros.

This change updates the `test` script in `package.json` to include the `--coverage` flag. This ensures that running the standard `npm test` command will generate the necessary coverage reports for the UI.
The test results dashboard was showing all zeros because it was trying to fetch data from a backend API instead of the local report files generated by the test runner. The API was returning an empty response, which caused the dashboard to display incorrect data.

This change modifies the `getTestResults` and `getCoverageData` functions in `testService.ts` to fetch the data directly from the static JSON files (`test-results.json` and `coverage-summary.json`). This ensures that the dashboard always reflects the results of the latest local test run.
The test results dashboard was showing "No Test Results Available" because of a mismatch between the data structure of the `test-results.json` file generated by `vitest` and the data structure expected by the dashboard component.

This change updates the `getTestResults` function in `testService.ts` to transform the JSON data into the correct shape before passing it to the UI. This ensures that the summary and suite-level test results are displayed correctly.
This commit addresses several issues causing React UI tests to fail.

The test suite `errors.test.tsx` had a test that was consistently timing out. The test was intended to simulate a timeout using fake timers, but the timer mocking was not working correctly in the test environment. After several attempts to fix the timer mocking, the test was refactored to use real timers and rely on react-testing-library's async `findBy` queries. This makes the test more robust and independent of the tricky fake timer setup.

The test suite `api.test.ts` had multiple failures related to API URL configuration.
1.  The `getApiUrl` function had its logic for handling development ports removed, causing tests that relied on this logic to fail. The logic was restored.
2.  A test checking for error-throwing behavior was failing because a module-level side effect was causing the `import()` to throw, which the test was not designed to catch. The test was updated to correctly assert that the promise from the dynamic import is rejected.
3.  The `MCPClientService` was not being imported correctly in the tests because the class itself was not exported. The class is now exported, and a duplicate import was removed from the service file.
@rnarciso rnarciso closed this Aug 19, 2025
@rnarciso rnarciso deleted the fix-failing-react-tests branch August 19, 2025 00:08
POWERFULMOVES added a commit to POWERFULMOVES/PMOVES-Archon that referenced this pull request Feb 12, 2026
coleam00 pushed a commit that referenced this pull request Apr 7, 2026
* feat: Phase 4 - Express to Hono migration

Replace Express with Hono in @archon/server for improved performance
and better Bun integration. This is a focused HTTP layer migration
that preserves all existing API functionality.

Changes:
- Replace express with hono dependency
- Migrate all endpoints to Hono context-based handlers
- Use Bun.serve() for native Bun server integration
- Update optional parameter syntax from Express 5 to Hono style
- Remove express.json()/express.raw() middleware (Hono handles natively)
- Change default port from 3000 to 3090
- Update log prefixes from [Express] to [Hono]
- Update CLAUDE.md documentation references

All endpoints verified working:
- GET /health, /health/db, /health/concurrency
- POST /test/message, GET /test/messages/:id
- DELETE /test/messages/:id?, PUT /test/mode
- POST /webhooks/github (signature verification)

* refactor: Simplify Hono endpoint handlers

- Remove unnecessary try/catch from /health/concurrency endpoint
  (getStats() is a simple sync operation that doesn't need error handling)
- Remove outer try/catch from /test/message endpoint
  (validation returns early, processing is fire-and-forget with its own error handler)
- Consolidate validation checks with clearer error messages
- Reduce nesting depth for improved readability

Total: 12 lines removed while maintaining identical functionality

* docs: Update port references from 3000 to 3090 after Hono migration

* fix: Improve error handling in Hono endpoints

- Add global app.onError() handler for consistent unhandled exception responses
- Add explicit JSON parsing error handling to /test/message endpoint (returns 400 instead of 500)
- Add explicit JSON parsing error handling to /test/mode endpoint (was completely unhandled)
- Fix CLAUDE.md worktree port example to use correct port (3637 with base 3090)
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
* feat: Phase 4 - Express to Hono migration

Replace Express with Hono in @archon/server for improved performance
and better Bun integration. This is a focused HTTP layer migration
that preserves all existing API functionality.

Changes:
- Replace express with hono dependency
- Migrate all endpoints to Hono context-based handlers
- Use Bun.serve() for native Bun server integration
- Update optional parameter syntax from Express 5 to Hono style
- Remove express.json()/express.raw() middleware (Hono handles natively)
- Change default port from 3000 to 3090
- Update log prefixes from [Express] to [Hono]
- Update CLAUDE.md documentation references

All endpoints verified working:
- GET /health, /health/db, /health/concurrency
- POST /test/message, GET /test/messages/:id
- DELETE /test/messages/:id?, PUT /test/mode
- POST /webhooks/github (signature verification)

* refactor: Simplify Hono endpoint handlers

- Remove unnecessary try/catch from /health/concurrency endpoint
  (getStats() is a simple sync operation that doesn't need error handling)
- Remove outer try/catch from /test/message endpoint
  (validation returns early, processing is fire-and-forget with its own error handler)
- Consolidate validation checks with clearer error messages
- Reduce nesting depth for improved readability

Total: 12 lines removed while maintaining identical functionality

* docs: Update port references from 3000 to 3090 after Hono migration

* fix: Improve error handling in Hono endpoints

- Add global app.onError() handler for consistent unhandled exception responses
- Add explicit JSON parsing error handling to /test/message endpoint (returns 400 instead of 500)
- Add explicit JSON parsing error handling to /test/mode endpoint (was completely unhandled)
- Fix CLAUDE.md worktree port example to use correct port (3637 with base 3090)
leex279 added a commit that referenced this pull request Apr 17, 2026
…nd JSDoc (#1152)

The server's getPort() fallback changed from 3000 to 3090 in the Hono
migration (#318), but .env.example, the setup wizard's generated .env,
and the JSDoc describing the fallback were not updated — leaving three
different sources of truth for "the default PORT."

When the wizard writes PORT=3000 to ~/.archon/.env (which the Hono
server loads with override: true, while Vite only reads repo-local
.env), the two processes can land on different ports silently. That
mismatch is the real mechanism behind the failure described in #1152.

- .env.example: comment out PORT, document 3090 as the default
- packages/cli/src/commands/setup.ts: wizard no longer writes PORT=3000
  into the generated .env; fix the "Additional Options" note
- packages/cli/src/commands/setup.test.ts: assert no bare PORT= line and
  the commented default is present
- packages/core/src/utils/port-allocation.ts: fix stale JSDoc "default
  3000" -> "default 3090"
- deploy/.env.example: keep Docker default at 3000 (compose/Caddy target
  that) but annotate it so users don't copy it for local dev

Single source of truth for the local-dev default is now basePort in
port-allocation.ts.
Wirasm pushed a commit that referenced this pull request Apr 17, 2026
…nd JSDoc (#1152) (#1271)

The server's getPort() fallback changed from 3000 to 3090 in the Hono
migration (#318), but .env.example, the setup wizard's generated .env,
and the JSDoc describing the fallback were not updated — leaving three
different sources of truth for "the default PORT."

When the wizard writes PORT=3000 to ~/.archon/.env (which the Hono
server loads with override: true, while Vite only reads repo-local
.env), the two processes can land on different ports silently. That
mismatch is the real mechanism behind the failure described in #1152.

- .env.example: comment out PORT, document 3090 as the default
- packages/cli/src/commands/setup.ts: wizard no longer writes PORT=3000
  into the generated .env; fix the "Additional Options" note
- packages/cli/src/commands/setup.test.ts: assert no bare PORT= line and
  the commented default is present
- packages/core/src/utils/port-allocation.ts: fix stale JSDoc "default
  3000" -> "default 3090"
- deploy/.env.example: keep Docker default at 3000 (compose/Caddy target
  that) but annotate it so users don't copy it for local dev

Single source of truth for the local-dev default is now basePort in
port-allocation.ts.
ztech-gthb pushed a commit to ztech-gthb/Archon that referenced this pull request Apr 18, 2026
…nd JSDoc (coleam00#1152) (coleam00#1271)

The server's getPort() fallback changed from 3000 to 3090 in the Hono
migration (coleam00#318), but .env.example, the setup wizard's generated .env,
and the JSDoc describing the fallback were not updated — leaving three
different sources of truth for "the default PORT."

When the wizard writes PORT=3000 to ~/.archon/.env (which the Hono
server loads with override: true, while Vite only reads repo-local
.env), the two processes can land on different ports silently. That
mismatch is the real mechanism behind the failure described in coleam00#1152.

- .env.example: comment out PORT, document 3090 as the default
- packages/cli/src/commands/setup.ts: wizard no longer writes PORT=3000
  into the generated .env; fix the "Additional Options" note
- packages/cli/src/commands/setup.test.ts: assert no bare PORT= line and
  the commented default is present
- packages/core/src/utils/port-allocation.ts: fix stale JSDoc "default
  3000" -> "default 3090"
- deploy/.env.example: keep Docker default at 3000 (compose/Caddy target
  that) but annotate it so users don't copy it for local dev

Single source of truth for the local-dev default is now basePort in
port-allocation.ts.
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
* feat: Phase 4 - Express to Hono migration

Replace Express with Hono in @archon/server for improved performance
and better Bun integration. This is a focused HTTP layer migration
that preserves all existing API functionality.

Changes:
- Replace express with hono dependency
- Migrate all endpoints to Hono context-based handlers
- Use Bun.serve() for native Bun server integration
- Update optional parameter syntax from Express 5 to Hono style
- Remove express.json()/express.raw() middleware (Hono handles natively)
- Change default port from 3000 to 3090
- Update log prefixes from [Express] to [Hono]
- Update CLAUDE.md documentation references

All endpoints verified working:
- GET /health, /health/db, /health/concurrency
- POST /test/message, GET /test/messages/:id
- DELETE /test/messages/:id?, PUT /test/mode
- POST /webhooks/github (signature verification)

* refactor: Simplify Hono endpoint handlers

- Remove unnecessary try/catch from /health/concurrency endpoint
  (getStats() is a simple sync operation that doesn't need error handling)
- Remove outer try/catch from /test/message endpoint
  (validation returns early, processing is fire-and-forget with its own error handler)
- Consolidate validation checks with clearer error messages
- Reduce nesting depth for improved readability

Total: 12 lines removed while maintaining identical functionality

* docs: Update port references from 3000 to 3090 after Hono migration

* fix: Improve error handling in Hono endpoints

- Add global app.onError() handler for consistent unhandled exception responses
- Add explicit JSON parsing error handling to /test/message endpoint (returns 400 instead of 500)
- Add explicit JSON parsing error handling to /test/mode endpoint (was completely unhandled)
- Fix CLAUDE.md worktree port example to use correct port (3637 with base 3090)
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…nd JSDoc (coleam00#1152) (coleam00#1271)

The server's getPort() fallback changed from 3000 to 3090 in the Hono
migration (coleam00#318), but .env.example, the setup wizard's generated .env,
and the JSDoc describing the fallback were not updated — leaving three
different sources of truth for "the default PORT."

When the wizard writes PORT=3000 to ~/.archon/.env (which the Hono
server loads with override: true, while Vite only reads repo-local
.env), the two processes can land on different ports silently. That
mismatch is the real mechanism behind the failure described in coleam00#1152.

- .env.example: comment out PORT, document 3090 as the default
- packages/cli/src/commands/setup.ts: wizard no longer writes PORT=3000
  into the generated .env; fix the "Additional Options" note
- packages/cli/src/commands/setup.test.ts: assert no bare PORT= line and
  the commented default is present
- packages/core/src/utils/port-allocation.ts: fix stale JSDoc "default
  3000" -> "default 3090"
- deploy/.env.example: keep Docker default at 3000 (compose/Caddy target
  that) but annotate it so users don't copy it for local dev

Single source of truth for the local-dev default is now basePort in
port-allocation.ts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant