Skip to content

Conversation

@github-actions
Copy link
Contributor

Goal and Rationale

This PR adds comprehensive unit tests for the useKeyboardShortcut.ts hook, which was previously untested (0% coverage).

Coverage area chosen: Simple utility hook in /react/src/hooks/useKeyboardShortcut.ts

Why it matters: This hook manages keyboard shortcuts throughout the application, preventing shortcuts from triggering when input fields are focused or modals are open. Testing it ensures:

  • Correct detection of focusable input elements (input, textarea, select)
  • Proper handling of Shadow DOM elements
  • Modal presence detection works correctly
  • Optional meta key filtering functions as expected
  • Edge cases like null activeElement are handled gracefully

Approach

Testing strategy:

  • Unit testing with Jest and React Testing Library's renderHook
  • Mock ahooks' useEventListener to intercept event registration
  • Test all conditional branches and edge cases
  • Verify Shadow DOM traversal logic
  • Test combined conditions (input focused + modal open, etc.)

Implementation steps:

  1. Created comprehensive test suite with 22 test cases
  2. Mocked ahooks.useEventListener to capture event handlers
  3. Tested basic functionality (handler invocation, event passing)
  4. Tested input element detection (input, textarea, select)
  5. Tested modal detection
  6. Tested skipShortcutOnMetaKey option with all modifier keys
  7. Tested Shadow DOM handling
  8. Tested combined conditions and edge cases
  9. Verified all tests pass independently and together

Impact Measurement

Test Coverage Results

Metric Before After Change
useKeyboardShortcut.ts Statements 0% 96% +96%
useKeyboardShortcut.ts Branches 0% 95.83% +95.83%
useKeyboardShortcut.ts Functions 0% 100% +100%
useKeyboardShortcut.ts Lines 0% 100% +100%
React Hooks Coverage (Statements) 8.64% 10.17% +1.53%
React Hooks Coverage (Branches) N/A 7.45% N/A
React Hooks Coverage (Functions) 6.65% 7.83% +1.18%
React Hooks Coverage (Lines) 8.67% 10.11% +1.44%
Overall React Coverage (Statements) 4.08% 4.27% +0.19%

Test Suite Results

  • Test Suites: 15 total (13 passed, 2 pre-existing failures unrelated to this PR)
  • Tests: 215 total (all passed) - up from 193
  • New Tests: 22 comprehensive test cases added
  • Test Execution Time: ~1.4 seconds for new tests

Note: The 2 failed test suites are pre-existing issues related to ES Module imports (nuqs package) and are unrelated to this PR.

Trade-offs

What changed:

  • Added test file: react/src/hooks/useKeyboardShortcut.test.ts (324 lines)
  • No production code changes
  • Test maintenance overhead: Low (well-structured hook with clear logic)

Complexity:

  • Tests are straightforward unit tests with mocked dependencies
  • Mock setup is simple (ahooks.useEventListener and DOM manipulation)
  • Follows existing test patterns in the codebase
  • Comprehensive coverage of all code paths

Validation

Testing approach:

# Run new tests specifically
cd react && NODE_OPTIONS='--no-deprecation --experimental-vm-modules' npx jest src/hooks/useKeyboardShortcut.test.ts

# Run full test suite with coverage
cd react && NODE_OPTIONS='--no-deprecation --experimental-vm-modules' npx jest --coverage

Success criteria met:
✅ All 22 new tests pass
✅ 96%+ coverage achieved for useKeyboardShortcut.ts
✅ 100% function and line coverage
✅ No existing tests broken
✅ Hooks coverage improved by 1.44-1.53%
✅ Overall React coverage improved by 0.19%

Test Cases Covered

Basic Functionality (2 tests)

  1. Handler invocation - Verifies handler is called when keyboard event is triggered
  2. Event passing - Confirms KeyboardEvent is passed to handler correctly

Input Element Detection (4 tests)

  1. Input focused - Prevents shortcut when input element is focused
  2. Textarea focused - Prevents shortcut when textarea is focused
  3. Select focused - Prevents shortcut when select element is focused
  4. Non-input focused - Allows shortcut when non-input element is focused

Modal Detection (2 tests)

  1. Modal open - Prevents shortcut when Ant Design modal is open
  2. No modal - Allows shortcut when no modal is present

skipShortcutOnMetaKey Option (6 tests)

  1. Ctrl key - Prevents shortcut when Ctrl is pressed with option enabled
  2. Meta key - Prevents shortcut when Meta is pressed with option enabled
  3. Alt key - Prevents shortcut when Alt is pressed with option enabled
  4. Shift key - Prevents shortcut when Shift is pressed with option enabled
  5. Without option - Allows shortcut with modifier keys when option disabled
  6. No modifiers - Allows shortcut with no modifier keys when option enabled

Shadow DOM Handling (2 tests)

  1. Input in shadow - Prevents shortcut when input inside shadow DOM is focused
  2. Non-input in shadow - Allows shortcut when non-input inside shadow DOM is focused

Combined Conditions (3 tests)

  1. Input + modal - Prevents shortcut when both input focused and modal open
  2. Input + meta + option - Prevents shortcut with all blocking conditions
  3. No blocks with option - Allows shortcut when no blocking conditions present

Edge Cases (3 tests)

  1. Null activeElement - Handles null activeElement gracefully
  2. Multiple events - Correctly handles multiple keyboard events in sequence
  3. Different key types - Works with various key types (Enter, Escape, Arrow keys)

Future Work

Additional coverage opportunities identified:

Based on the baseline coverage analysis and previous PR #5147 recommendations, the next priority areas are:

  1. More Simple Utility Hooks (0% coverage, high ROI):

    • useHighlight.ts - Code syntax highlighting with shiki (already reviewed, good candidate)
    • useScrollBreackPoint.tsx - Scroll position tracking (already reviewed, good candidate)
  2. Validation Hooks (important business logic):

    • useValidateServiceName.tsx - Service name validation
    • useValidateSessionName.tsx - Session name validation
  3. State Management Hooks:

    • useLocalStorageGlobalState.tsx - Browser storage hook
    • useHiddenColumnKeysSetting.tsx - UI settings persistence
  4. Backend.AI UI Package (packages/backend.ai-ui/src/helper/index.ts):

    • Currently at 63.08% statement coverage
    • Focus on untested functions to reach 80%+ coverage

Reproducibility

Setup Commands

# Install dependencies
pnpm install --ignore-scripts

# Navigate to React project
cd react

Run Tests

# Run only useKeyboardShortcut tests
NODE_OPTIONS='--no-deprecation --experimental-vm-modules' npx jest src/hooks/useKeyboardShortcut.test.ts

# Run all tests with coverage
NODE_OPTIONS='--no-deprecation --experimental-vm-modules' npx jest --coverage

Measurement Procedures

  1. Coverage is collected via Jest's built-in coverage tool
  2. Results are reported per-file and aggregated by directory
  3. Coverage metrics: Statements, Branches, Functions, Lines
  4. Baseline comparison from PR Daily Test Coverage Improver - Add tests for useVariantConfigs hook #5147 coverage numbers

Expected Results Format

src/hooks
  useKeyboardShortcut.ts    | 96 | 95.83 | 100 | 100 | 21

Related: Discussion #4760 - Daily Test Coverage Improver Research and Plan

AI generated by Daily Test Coverage Improver

AI generated by Daily Test Coverage Improver

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.

0 participants