[WIP] Add tests for object table and reporting components#309
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the MSW (Mock Service Worker) testing infrastructure established in PR #308 to cover additional ObjectUI components and server-driven definitions. It adds comprehensive test suites for ObjectGrid, Dashboard, and various server-driven layout schemas (app, page, and component composition).
Changes:
- Adds three new test files covering ObjectGrid CRUD operations, Dashboard widgets, and server-driven schema definitions
- Updates package dependencies to include @object-ui/components, @object-ui/plugin-dashboard, and @object-ui/plugin-grid
- Configures component registration in vitest.setup.ts to ensure all necessary components (div, text, grid, dashboard) are available during tests
- Updates build configuration to include plugin-dashboard in the pretest build step
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace dependencies for components, plugin-dashboard, and plugin-grid packages to the msw-object-form example |
| package.json | Adds packageManager field and includes plugin-dashboard in pretest build script |
| vitest.setup.ts | Imports component packages to register div, text, grid, and dashboard components for testing |
| ServerDefinitions.test.tsx | Tests server-driven app layouts, page definitions, and nested component composition using SchemaRenderer |
| ObjectGrid.test.tsx | Tests ObjectGrid rendering, column configuration, CRUD operations, and data loading (contains critical initialization bug) |
| Dashboard.test.tsx | Tests Dashboard layout rendering, widget components, and data-driven metrics |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
|
||
| import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; | ||
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; |
There was a problem hiding this comment.
The userEvent import is unused. This import should be removed unless you plan to add user interaction tests (e.g., clicking buttons, selecting rows) in the near future.
| import userEvent from '@testing-library/user-event'; |
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
| import { SchemaRenderer } from '@object-ui/react'; | ||
| import type { AppSchema, PageSchema } from '@object-ui/types'; |
There was a problem hiding this comment.
The PageSchema import is unused. It should be removed to keep imports clean.
| import type { AppSchema, PageSchema } from '@object-ui/types'; | |
| import type { AppSchema } from '@object-ui/types'; |
|
|
||
| // Get actual data from server | ||
| const contacts = await driver!.find('contact', {}); |
There was a problem hiding this comment.
Using the non-null assertion operator (!) on getDriver() is unsafe. If the driver is null for any reason (e.g., server initialization fails), this will cause a runtime error.
Consider adding a null check:
const driver = getDriver();
if (!driver) {
throw new Error('Driver not initialized');
}
const contacts = await driver.find('contact', {});This makes the test more robust and provides clearer error messages if something goes wrong during setup.
| // Get actual data from server | |
| const contacts = await driver!.find('contact', {}); | |
| if (!driver) { | |
| throw new Error('Driver not initialized'); | |
| } | |
| // Get actual data from server | |
| const contacts = await driver.find('contact', {}); |
|
|
||
| describe('ObjectGrid MSW Integration', () => { | ||
| let dataSource: ObjectStackDataSource; | ||
|
|
||
| beforeAll(async () => { | ||
| await startMockServer(); | ||
| dataSource = new ObjectStackDataSource(); |
There was a problem hiding this comment.
The ObjectStackDataSource constructor requires a client parameter but is being instantiated without arguments. This will cause a runtime error when the dataSource tries to access the client.
Looking at the working ObjectForm tests in the same directory, you need to:
- Declare a
client: ObjectStackClientvariable - Initialize it in beforeAll with
new ObjectStackClient({ baseUrl: 'http://localhost:3000' })and callawait client.connect() - Pass it to the dataSource:
dataSource = new ObjectStackDataSource(client)
Reference: examples/msw-object-form/src/tests/ObjectForm.test.tsx:25-28
| describe('ObjectGrid MSW Integration', () => { | |
| let dataSource: ObjectStackDataSource; | |
| beforeAll(async () => { | |
| await startMockServer(); | |
| dataSource = new ObjectStackDataSource(); | |
| import { ObjectStackClient } from '@objectstack/client'; | |
| describe('ObjectGrid MSW Integration', () => { | |
| let dataSource: ObjectStackDataSource; | |
| let client: ObjectStackClient; | |
| beforeAll(async () => { | |
| await startMockServer(); | |
| client = new ObjectStackClient({ baseUrl: 'http://localhost:3000' }); | |
| await client.connect(); | |
| dataSource = new ObjectStackDataSource(client); |
| it('should trigger onDelete callback', async () => { | ||
| const onDelete = vi.fn(); | ||
|
|
||
| render( | ||
| <ObjectGrid | ||
| schema={{ | ||
| type: 'object-grid', | ||
| objectName: 'contact', | ||
| columns: ['name', 'email'], | ||
| actions: true, | ||
| }} | ||
| dataSource={dataSource} | ||
| onDelete={onDelete} | ||
| /> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| }, { timeout: 5000 }); | ||
|
|
||
| // Look for delete buttons (implementation-specific) | ||
| // This is a placeholder for when delete actions are implemented | ||
| }); |
There was a problem hiding this comment.
This test is incomplete - it renders the grid with delete callback but doesn't actually test that clicking a delete button triggers the callback. The comment "This is a placeholder for when delete actions are implemented" indicates this is work in progress.
To complete this test, you should:
- Find and click a delete button in the rendered grid
- Verify that onDelete was called with the expected record data
Alternatively, if delete actions are not yet implemented in ObjectGrid, consider skipping this test with it.skip() or it.todo() to clearly indicate it's not yet functional.
| it('should show loading state initially', async () => { | ||
| const { container } = render( | ||
| <ObjectGrid | ||
| schema={{ | ||
| type: 'object-grid', | ||
| objectName: 'contact', | ||
| columns: ['name', 'email'], | ||
| }} | ||
| dataSource={dataSource} | ||
| /> | ||
| ); | ||
|
|
||
| // Check for loading indicator (implementation-specific) | ||
| // The grid should eventually show data | ||
| await waitFor(() => { | ||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| }, { timeout: 5000 }); | ||
| }); |
There was a problem hiding this comment.
This test claims to check for a loading state but doesn't actually verify that a loading indicator appears. It only waits for the data to eventually load. The comment "Check for loading indicator (implementation-specific)" suggests this is incomplete.
To properly test loading states, you should:
- Check for a loading indicator immediately after render (before data loads)
- Then verify the loading indicator disappears and data appears
If the ObjectGrid component doesn't currently show a loading state, consider marking this test with it.skip() or it.todo(), or rename it to better reflect what it actually tests (e.g., "should eventually load and display data").
|
|
||
| describe('ObjectGrid MSW Integration', () => { | ||
| let dataSource: ObjectStackDataSource; | ||
|
|
||
| beforeAll(async () => { | ||
| await startMockServer(); | ||
| dataSource = new ObjectStackDataSource(); |
There was a problem hiding this comment.
Missing import for ObjectStackClient. The ObjectStackDataSource requires an ObjectStackClient instance to be passed to its constructor.
Add this import:
import { ObjectStackClient } from '@objectstack/client';Reference: examples/msw-object-form/src/tests/ObjectForm.test.tsx:11
| describe('ObjectGrid MSW Integration', () => { | |
| let dataSource: ObjectStackDataSource; | |
| beforeAll(async () => { | |
| await startMockServer(); | |
| dataSource = new ObjectStackDataSource(); | |
| import { ObjectStackClient } from '@objectstack/client'; | |
| describe('ObjectGrid MSW Integration', () => { | |
| let dataSource: ObjectStackDataSource; | |
| beforeAll(async () => { | |
| await startMockServer(); | |
| const client = new ObjectStackClient(); | |
| dataSource = new ObjectStackDataSource(client); |
| it('should support row selection', async () => { | ||
| const onRowSelect = vi.fn(); | ||
|
|
||
| render( | ||
| <ObjectGrid | ||
| schema={{ | ||
| type: 'object-grid', | ||
| objectName: 'contact', | ||
| columns: ['name', 'email'], | ||
| enableSelection: true, | ||
| }} | ||
| dataSource={dataSource} | ||
| onRowSelect={onRowSelect} | ||
| /> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| }, { timeout: 5000 }); | ||
|
|
||
| // Note: Row selection UI depends on the actual grid implementation | ||
| // This is a placeholder for when selection is implemented | ||
| }); |
There was a problem hiding this comment.
This test is incomplete - it renders the grid with row selection enabled and waits for data to load, but doesn't actually verify that row selection works. The comment "This is a placeholder for when selection is implemented" indicates this is work in progress.
To complete this test, you should:
- Find and click on a row selection checkbox/control
- Verify that onRowSelect was called with the expected data
- Verify visual feedback for the selected row
Alternatively, if row selection is not yet implemented in ObjectGrid, consider skipping this test with it.skip() or it.todo() to clearly indicate it's not yet functional.
| it('should trigger onEdit callback', async () => { | ||
| const onEdit = vi.fn(); | ||
|
|
||
| render( | ||
| <ObjectGrid | ||
| schema={{ | ||
| type: 'object-grid', | ||
| objectName: 'contact', | ||
| columns: ['name', 'email'], | ||
| actions: true, | ||
| }} | ||
| dataSource={dataSource} | ||
| onEdit={onEdit} | ||
| /> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| }, { timeout: 5000 }); | ||
|
|
||
| // Look for edit buttons (implementation-specific) | ||
| // This is a placeholder for when edit actions are implemented | ||
| }); |
There was a problem hiding this comment.
This test is incomplete - it renders the grid with edit callback but doesn't actually test that clicking an edit button triggers the callback. The comment "This is a placeholder for when edit actions are implemented" indicates this is work in progress.
To complete this test, you should:
- Find and click an edit button in the rendered grid
- Verify that onEdit was called with the expected record data
Alternatively, if edit actions are not yet implemented in ObjectGrid, consider skipping this test with it.skip() or it.todo() to clearly indicate it's not yet functional.
Extend MSW Testing Infrastructure
Building on PR #308, extend test coverage to additional components and server-driven definitions:
Test Results Summary
Overall: 23/38 tests passing (60.5%) 📊
Passing:
Remaining Issues:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.