Skip to content

Conversation

sadpandajoe
Copy link
Member

SUMMARY

Removes 3 duplicate JSX test files that have TypeScript counterparts already in the codebase. This cleanup aligns with the frontend modernization effort to eliminate JavaScript files.

Files removed:

  1. ListView.test.jsx - All tests merged into existing ListView.test.tsx (now 9 tests total)
  2. DatasourceControl.test.jsx - Duplicate of comprehensive DatasourceControl.test.tsx (21 tests)
  3. VizTypeControl.test.jsx - Duplicate of comprehensive VizTypeControl.test.tsx (10 tests)

Impact:

  • 4 files changed, 259 insertions(+), 575 deletions(-)
  • Net reduction: 316 lines
  • All 40 tests passing ✅

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Verify all migrated tests pass:
npm run test -- src/components/ListView/ListView.test.tsx
npm run test -- src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
npm run test -- src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

- Merged ListView.test.jsx comprehensive tests into ListView.test.tsx (9 tests total)
- Removed DatasourceControl.test.jsx (all tests already in .tsx with 21 tests)
- Removed VizTypeControl.test.jsx (all tests already in .tsx with 10 tests)

All 40 tests pass. TypeScript migration aligns with frontend modernization goals.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I've completed my review and didn't find any issues.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@dosubot dosubot bot added the change:frontend Requires changing the frontend label Oct 10, 2025
- Added MockedListViewProps type for proper Jest mock typing
- Replaced string literal operators with ListViewFilterOperator enum values
- Fixed fetchSelectsMock to return proper Promise structure
- Properly typed fetchData mock with jest.fn<> generic parameters
- Changed bulk action 'style' property to 'type' (correct interface)
- Removed 'title' property (not in ListViewProps interface)
- Created factory function with explicit overrides pattern

All 9 ListView tests pass with full TypeScript type safety.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant