Skip to content

feat: fix new cdp tests for tools#358

Merged
shadowfax92 merged 3 commits intomainfrom
feat/fix-new-cdp-tests
Feb 24, 2026
Merged

feat: fix new cdp tests for tools#358
shadowfax92 merged 3 commits intomainfrom
feat/fix-new-cdp-tests

Conversation

@shadowfax92
Copy link
Contributor

  • feat: new tools tests
  • fix: lint warnings by disabling or TODO

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 24, 2026

Greptile Summary

Major test suite refactoring that consolidates CDP-based tests into a unified structure. The PR removes ~5,000+ lines of old controller-based and CDP test files, replacing them with ~1,000 lines of new streamlined CDP tests using a shared withBrowser helper.

Key changes:

  • New test infrastructure: Created with-browser.ts helper that provides cached CDP browser connection with mutex for concurrent test execution
  • Test consolidation: Removed separate tests/tools/cdp/ and tests/tools/controller/ directories, replaced with unified tests/tools/ tests
  • Package scripts updated: Consolidated test commands from separate test:cdp and test:controller into single test:tools command
  • Bug fix in browser.ts: Added retry logic (10 attempts, 100ms delay) when fetching tab info after creation to handle race conditions
  • Lint warnings addressed: Added biome-ignore comments with TODO notes for excessive complexity and any types in GraphQL code
  • Minor cleanup: Updated GitHub Actions workflow, bumped Biome schema version

The refactoring maintains test coverage while significantly reducing code duplication and improving test helper reusability.

Confidence Score: 4/5

  • Safe to merge with minor cleanup recommended for commented code
  • Large refactoring successfully consolidates test infrastructure and reduces duplication. The new withBrowser helper and retry logic in browser.ts improve test reliability. Minor issues include leftover commented-out code and silent error swallowing in retry loop. The TODO comments acknowledge technical debt appropriately.
  • apps/agent/web-ext.config.ts needs commented code removed

Important Files Changed

Filename Overview
apps/agent/web-ext.config.ts Added commented-out line with incorrect syntax, appears to be leftover debugging code
apps/server/package.json Consolidated test scripts, removed separate cdp/controller test commands in favor of unified test:tools
apps/server/src/browser/browser.ts Added retry logic with 10 attempts and 100ms delay when fetching tab info after creation
apps/server/tests/helpers/utils.ts Removed license header, removed withCdpBrowser wrapper and mock helper functions
apps/server/tests/helpers/with-browser.ts New test helper providing cached CDP browser connection with mutex for concurrent test execution
apps/server/tests/tools/input.test.ts New CDP-based input test with 252 lines covering fill, click, check/uncheck, select, hover, scroll, and keyboard
apps/server/tests/tools/navigation.test.ts New CDP-based navigation test with 114 lines covering pages, navigation, and wait_for
apps/server/tests/tools/observation.test.ts New CDP-based observation test with 131 lines covering screenshots and snapshots

Last reviewed commit: 1d4763e

@shadowfax92 shadowfax92 merged commit cb8aa6c into main Feb 24, 2026
6 of 7 checks passed
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.

1 participant