Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions apps/console/src/__tests__/BrowserSimulation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,79 @@ describe('Console Application Simulation', () => {
expect(screen.getAllByText('5000').length).toBeGreaterThan(0);
});

// -----------------------------------------------------------------------------
// SIMPLIFIED FORM INTEGRATION TESTS
// -----------------------------------------------------------------------------
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The Form and Grid integration test sections lack the header comment block that documents the test scenarios covered, unlike the Kanban Integration section (lines 295-303). For consistency with the established pattern, these sections should have similar header comments describing what scenarios they cover. This improves code documentation and makes the test structure clearer.

Suggested change
// -----------------------------------------------------------------------------
// -----------------------------------------------------------------------------
// These tests verify:
// 1. Metadata-driven form generation using MockDataSource.getObjectSchema
// 2. That the "New Kitchen Sink" action opens the form dialog correctly
// 3. That the schema-based form renders without runtime errors
// -----------------------------------------------------------------------------

Copilot uses AI. Check for mistakes.
it('Form Scenario B: Metadata-Driven Form Generation', async () => {
vi.spyOn(mocks.MockDataSource.prototype, 'getObjectSchema').mockResolvedValue({
name: 'kitchen_sink',
fields: {
name: { type: 'text', label: 'Name Field' },
amount: { type: 'number', label: 'Amount Field' }
}
});
Comment on lines +224 to +230
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The mock setup for getObjectSchema at line 224 will override the default mock behavior defined in the MockDataSource class (lines 34-49), but only for this specific test. However, this mock is not cleaned up or restored after the test completes. While Vitest typically handles this automatically with auto-cleanup, it's good practice to either use mockResolvedValueOnce for single-use mocks or ensure the spy is restored if other tests depend on the default behavior. The existing Kanban tests (e.g., line 427) use the same pattern, so this is consistent with the codebase.

Copilot uses AI. Check for mistakes.

renderApp('/kitchen_sink');
await waitFor(() => {
expect(screen.getByRole('heading', { name: /Kitchen Sink/i })).toBeInTheDocument();
});

// Verify the form can be opened (showing metadata was loaded)
const newButton = screen.getByRole('button', { name: /New Kitchen Sink/i });
fireEvent.click(newButton);

// Verify form loaded with schema-based fields
await waitFor(() => {
expect(screen.getByRole('dialog')).toBeInTheDocument();
});

// Form should render based on mocked schema
// The actual field labels might differ based on implementation
// but the form should render without errors
expect(screen.queryByText(/error/i)).not.toBeInTheDocument();
});
Comment on lines +223 to +250
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The Form Scenario B test mocks getObjectSchema with custom field definitions (Name Field, Amount Field) but then doesn't verify that these fields actually appear in the rendered form. The test only checks for the absence of error messages, which is a weak assertion. According to the test philosophy in the PR description, Scenario B should "Assert: Check that the UI (Columns/Cards) was generated purely from the Mocked Schema return value." The test should verify that fields with labels "Name Field" and "Amount Field" are actually rendered.

Copilot uses AI. Check for mistakes.

// -----------------------------------------------------------------------------
// SIMPLIFIED GRID INTEGRATION TESTS
Comment on lines +220 to +253
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The comment headers say "SIMPLIFIED FORM INTEGRATION TESTS" and "SIMPLIFIED GRID INTEGRATION TESTS", but the PR description doesn't mention these being "simplified" versions. The word "SIMPLIFIED" suggests these are placeholder or incomplete tests, which aligns with the observation that they don't follow the complete 4-scenario structure (A-D) described in the PR. Consider either removing "SIMPLIFIED" and implementing complete test coverage, or clarifying in the PR description why these tests are intentionally simplified.

Copilot uses AI. Check for mistakes.
// -----------------------------------------------------------------------------
it('Grid Scenario A: Grid Rendering and Actions', async () => {
Comment on lines +223 to +255
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The test names "Form Scenario B" and "Grid Scenario A/B" don't match the established naming convention from the Kanban Integration tests, which use the full descriptive format like "Scenario A: Protocol Compliance & Rendering (Static Test)" or "Scenario B: Metadata-Driven Hydration (Server Test)". The Form and Grid tests should follow the same pattern for consistency. For example, "Form Scenario B" should be "Scenario B: Metadata-Driven Form Generation" to match the pattern.

Copilot uses AI. Check for mistakes.
renderApp('/kitchen_sink');

await waitFor(() => {
expect(screen.getByRole('heading', { name: /Kitchen Sink/i })).toBeInTheDocument();
});

const newButton = screen.getByRole('button', { name: /New Kitchen Sink/i });
expect(newButton).toBeInTheDocument();
});
Comment on lines +255 to +264
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The test description says "Grid Scenario A: Grid Rendering and Actions" but the test only verifies that a button exists. It doesn't actually test any actions (e.g., click handlers, button functionality). According to the PR's testing philosophy, "Event Binding: Verify that click handlers defined in events are attached (e.g., spy on the ActionRunner)." This test should either verify action handling or be renamed to just "Grid Rendering" without mentioning actions.

Copilot uses AI. Check for mistakes.

it('Grid Scenario B: Grid Data Loading', async () => {
const seedData = [
{ id: '1', name: 'Item 1', amount: 100 },
{ id: '2', name: 'Item 2', amount: 200 }
];

const findSpy = vi.spyOn(mocks.MockDataSource.prototype, 'find')
.mockResolvedValue({ data: seedData });

renderApp('/kitchen_sink');

await waitFor(() => {
expect(screen.getByRole('heading', { name: /Kitchen Sink/i })).toBeInTheDocument();
});

// Verify data source was called to load grid data
await waitFor(() => {
expect(findSpy).toHaveBeenCalledWith('kitchen_sink', expect.any(Object));
});

// Verify grid displays the loaded data
await waitFor(() => {
expect(screen.getByText('Item 1')).toBeInTheDocument();
});
expect(screen.getByText('Item 2')).toBeInTheDocument();
});
Comment on lines +220 to +291
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

According to the PR description and test philosophy, integration tests should follow a "4-scenario structure" (A-D): Protocol Compliance, Metadata Hydration, Business Operations (CRUD), and Dynamic Behavior (Expression Test). The Form tests only include Scenario B, and the Grid tests only include Scenarios A and B. This is incomplete coverage compared to the established Kanban Integration tests which implement all four scenarios. Consider adding the missing scenarios or documenting why they're not applicable.

Copilot uses AI. Check for mistakes.

});

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -664,3 +737,87 @@ describe('Kanban Integration', () => {
expect(findSpy).toHaveBeenCalledWith('project_task', expect.any(Object));
});
});


// -----------------------------------------------------------------------------
// FIELDS INTEGRATION TESTS
// -----------------------------------------------------------------------------
describe('Fields Integration', () => {
it('Scenario A: Field Type Mapping', async () => {
const { mapFieldTypeToFormType } = await import('@object-ui/fields');

expect(mapFieldTypeToFormType('text')).toBe('field:text');
expect(mapFieldTypeToFormType('email')).toBe('field:email');
expect(mapFieldTypeToFormType('number')).toBe('field:number');
expect(mapFieldTypeToFormType('boolean')).toBe('field:boolean');
expect(mapFieldTypeToFormType('select')).toBe('field:select');
});
Comment on lines +746 to +754
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The PR description claims to test "Field type mapping (textfield:text, picklistfield:select)", but the Fields Integration tests (lines 746-754) don't include a test case for 'picklist' mapping to 'field:select'. The test only covers 'text', 'email', 'number', 'boolean', and 'select', but not 'picklist'. This is an important field type mentioned in the schema definitions throughout the codebase (see lines 407, 416, 547, 548, 698, 699). Add a test case for picklist mapping.

Copilot uses AI. Check for mistakes.

it('Scenario A.2: Unknown Field Type Fallback in Form', async () => {
const { mapFieldTypeToFormType } = await import('@object-ui/fields');

// Verify unknown types fallback to text
expect(mapFieldTypeToFormType('unknown_type')).toBe('field:text');
expect(mapFieldTypeToFormType('custom_widget')).toBe('field:text');

// This ensures forms don't break when encountering unknown field types
// The actual rendering is tested via the full form integration tests
});
Comment on lines +756 to +765
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The test verifies unknown field types fallback to 'field:text', but doesn't actually test the end-to-end rendering behavior as mentioned in the comment "The actual rendering is tested via the full form integration tests". However, there are no full form integration tests in this PR that verify unknown field types render correctly in forms. This creates a gap in test coverage - the mapping is tested but not the actual rendering behavior.

Copilot uses AI. Check for mistakes.

it('Scenario B: Field Formatting Utilities', async () => {
const { formatCurrency, formatDate, formatPercent } = await import('@object-ui/fields');

const formatted = formatCurrency(1234.56);
expect(formatted).toContain('1,234.56');

const dateStr = formatDate(new Date('2024-01-15'));
expect(dateStr).toContain('2024');

const percent = formatPercent(0.1234);
expect(percent).toBe('12.34%');
Comment on lines +771 to +777
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The field formatting tests make assumptions about locale-specific formatting. The formatCurrency test checks for '1,234.56' which assumes 'en-US' locale, and formatDate checks for '2024' in the output. These tests could fail in different locales or timezone configurations. Consider using more robust assertions that account for locale variations, or explicitly mock the Intl API to ensure consistent test behavior across environments.

Suggested change
expect(formatted).toContain('1,234.56');
const dateStr = formatDate(new Date('2024-01-15'));
expect(dateStr).toContain('2024');
const percent = formatPercent(0.1234);
expect(percent).toBe('12.34%');
expect(typeof formatted).toBe('string');
// Strip all non-digit characters and verify the numeric content is correct,
// regardless of locale-specific separators or currency symbols.
expect(formatted.replace(/[^\d]/g, '')).toBe('123456');
const dateStr = formatDate(new Date('2024-01-15T00:00:00Z'));
expect(typeof dateStr).toBe('string');
// Ensure a 4-digit year is present without assuming a specific locale format.
expect(dateStr).toMatch(/\d{4}/);
const percent = formatPercent(0.1234);
expect(typeof percent).toBe('string');
// Accept both "12.34%" and "12,34 %" style outputs (different locales may use
// different decimal separators and optional spaces before the percent sign).
expect(percent).toMatch(/^12([.,]\d+)? ?%$/);

Copilot uses AI. Check for mistakes.
});
Comment on lines +746 to +778
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The Fields Integration tests use dynamic imports (await import) for testing pure utility functions like mapFieldTypeToFormType and formatCurrency. This is inconsistent with how these functions should be tested - they're pure functions with no side effects and should be imported statically at the top of the file like other test dependencies. The dynamic imports add unnecessary complexity and async overhead for synchronous functions. Reference: lines 2-5 show static imports for testing utilities.

Copilot uses AI. Check for mistakes.
});

// -----------------------------------------------------------------------------
// DASHBOARD INTEGRATION TESTS
// -----------------------------------------------------------------------------
describe('Dashboard Integration', () => {
const renderApp = (initialRoute: string) => {
return render(
<MemoryRouter initialEntries={[initialRoute]}>
<AppContent />
</MemoryRouter>
);
};
Comment on lines +785 to +791
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The renderApp helper function is defined locally within the Dashboard Integration test suite, but the same function already exists as a helper in the parent "Console Application Simulation" test suite (lines 99-105). This creates unnecessary duplication and potential confusion. Consider removing this duplicate definition and using the parent suite's helper, or if this suite needs to be independent, ensure it's clearly scoped.

Suggested change
const renderApp = (initialRoute: string) => {
return render(
<MemoryRouter initialEntries={[initialRoute]}>
<AppContent />
</MemoryRouter>
);
};

Copilot uses AI. Check for mistakes.

it('Scenario A: Dashboard Page Rendering', async () => {
renderApp('/dashboard/sales_dashboard');

await waitFor(() => {
expect(screen.getByText(/Sales Overview/i)).toBeInTheDocument();
});

expect(screen.getByText(/Sales by Region/i)).toBeInTheDocument();
});

it('Scenario B: Report Page Rendering', async () => {
renderApp('/page/report_page');

await waitFor(() => {
expect(screen.getByText(/Sales Performance Report/i)).toBeInTheDocument();
});

expect(screen.getByText('Region')).toBeInTheDocument();
expect(screen.getByText('North')).toBeInTheDocument();
});

Comment on lines +793 to +813
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The Dashboard Integration tests duplicate existing functionality from the "Console Application Simulation" test suite. Specifically, "Scenario 2: Dashboard Rendering (Sales Dashboard)" (lines 117-132) already tests dashboard page rendering, and "Scenario 5: Report Rendering (Report Page)" (lines 199-218) already tests report page rendering. The new tests (lines 793-812) are essentially duplicates with slightly different assertions. This violates DRY principles and creates maintenance overhead.

Suggested change
it('Scenario A: Dashboard Page Rendering', async () => {
renderApp('/dashboard/sales_dashboard');
await waitFor(() => {
expect(screen.getByText(/Sales Overview/i)).toBeInTheDocument();
});
expect(screen.getByText(/Sales by Region/i)).toBeInTheDocument();
});
it('Scenario B: Report Page Rendering', async () => {
renderApp('/page/report_page');
await waitFor(() => {
expect(screen.getByText(/Sales Performance Report/i)).toBeInTheDocument();
});
expect(screen.getByText('Region')).toBeInTheDocument();
expect(screen.getByText('North')).toBeInTheDocument();
});

Copilot uses AI. Check for mistakes.
it('Scenario C: Component Registry Check', async () => {
const { ComponentRegistry } = await import('@object-ui/core');

const dashboardRenderer = ComponentRegistry.get('dashboard');
expect(dashboardRenderer).toBeDefined();

const reportRenderer = ComponentRegistry.get('report');
expect(reportRenderer).toBeDefined();
});
Comment on lines +813 to +822
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The Component Registry Check test uses dynamic imports but doesn't follow the established pattern for integration tests. According to the PR description's test philosophy, integration tests should verify "Black Box" behavior - how components render from JSON schemas and interact with data. Testing the ComponentRegistry directly is a unit test concern, not an integration test. This test belongs in a separate unit test file for the @object-ui/core package, not in the integration test suite.

Suggested change
it('Scenario C: Component Registry Check', async () => {
const { ComponentRegistry } = await import('@object-ui/core');
const dashboardRenderer = ComponentRegistry.get('dashboard');
expect(dashboardRenderer).toBeDefined();
const reportRenderer = ComponentRegistry.get('report');
expect(reportRenderer).toBeDefined();
});

Copilot uses AI. Check for mistakes.
});
Loading