Skip to content

ci(react): add browser-based integration testing to a2ui_explorer#1607

Draft
ditman wants to merge 14 commits into
a2ui-project:mainfrom
ditman:react-add-integration-test-harness
Draft

ci(react): add browser-based integration testing to a2ui_explorer#1607
ditman wants to merge 14 commits into
a2ui-project:mainfrom
ditman:react-add-integration-test-harness

Conversation

@ditman

@ditman ditman commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds a karma + jasmine + headless chrome integration test harness to the React a2ui_explorer test app, similar to what we did with Lit here:

Key changes

  • Made the a2ui_explorer react app a stand-alone package, so we can customize its dependencies (and tests) separately from the react renderer itself
    • Also added some tweaks to the app so it can be inspected by the tests
  • Added karma+jasmine+headless chrome to run integration tests
  • Added a sample unit test to verify the setup works

Pre-launch Checklist

If you need help, consider asking for advice on the discussion board.

@ditman

ditman commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

I forgot to split the testing for react in unit vs integration in the github workflow!

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the React explorer application to statically generate and map specification examples instead of using Vite's dynamic import.meta.glob, enabling integration testing with Karma and Jasmine. It introduces a static generation script, configures Karma, adds a robust integration test suite, and refactors the main application component to support testing properties. The reviewer feedback highlights two key improvements: adding tsconfig.spec.json to the test:base wireit task files to ensure proper cache invalidation, and safely validating the initialExampleId in App.tsx to avoid potential runtime crashes from non-null assertions.

Comment thread renderers/react/a2ui_explorer/package.json
Comment thread renderers/react/a2ui_explorer/src/App.tsx
@ditman

ditman commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

(This commit was a little bit weird: 52d4975)

@ditman

ditman commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

@ditman

ditman commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review again

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the React Explorer to support integration testing with Karma and esbuild by replacing Vite-specific dynamic imports with a statically generated examples mapping. It also introduces new integration tests and test utilities. Feedback on the changes highlights a potential runtime crash in App.tsx if an invalid initialExampleId is provided, as the non-null assertion could lead to a TypeError when accessing selectedItem.messages. A fallback to the first demo item is suggested to prevent this issue.

Comment on lines +81 to +82
const [selectedExampleId, setSelectedExampleId] = useState(initialExampleId ?? demoItems[0].id);
const selectedItem = demoItems.find(e => e.id === selectedExampleId)!;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If initialExampleId is invalid or not found in demoItems, demoItems.find will return undefined. The non-null assertion ! will bypass TypeScript compilation checks, but at runtime, accessing selectedItem.messages will throw a TypeError: Cannot read properties of undefined (reading 'messages') and crash the application.

To prevent this, we should safely fall back to the first demo item (demoItems[0]) if the specified ID is not found, and use optional chaining for the initial state.

Suggested change
const [selectedExampleId, setSelectedExampleId] = useState(initialExampleId ?? demoItems[0].id);
const selectedItem = demoItems.find(e => e.id === selectedExampleId)!;
const [selectedExampleId, setSelectedExampleId] = useState(initialExampleId ?? demoItems[0]?.id);
const selectedItem = demoItems.find(e => e.id === selectedExampleId) ?? demoItems[0];

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.

1 participant