Fix test timeout in plugin-editor from Monaco Editor initialization#312
Fix test timeout in plugin-editor from Monaco Editor initialization#312
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…o 30s Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix test timeout issues in the @object-ui/plugin-editor package by increasing the beforeAll hook timeout from the default 10 seconds to 30 seconds. The change is motivated by the assumption that Monaco Editor initialization is causing the timeout.
Changes:
- Added a 30-second timeout to the
beforeAllhook in plugin-editor tests - Added a comment explaining that Monaco Editor is a heavy library that takes time to load
| beforeAll(async () => { | ||
| await import('./index'); | ||
| }); | ||
| }, 30000); // 30 second timeout for Monaco Editor initialization |
There was a problem hiding this comment.
No other plugin packages in the codebase use extended timeouts for their beforeAll hooks, even though packages like @object-ui/plugin-aggrid and @object-ui/plugin-charts also import heavy third-party libraries (AG Grid, Recharts).
This inconsistency suggests either:
- The timeout isn't actually necessary (the lazy loading is working as designed)
- Other plugins may have similar issues that haven't been encountered yet
Consider verifying whether this timeout is actually solving the problem or if there's a different root cause that should be addressed consistently across all plugins.
| // Note: Monaco Editor is a heavy library that takes time to load | ||
| beforeAll(async () => { | ||
| await import('./index'); | ||
| }); | ||
| }, 30000); // 30 second timeout for Monaco Editor initialization |
There was a problem hiding this comment.
The comment and timeout may be based on an incorrect assumption. Monaco Editor is lazy-loaded via React.lazy(() => import('./MonacoImpl')) in index.tsx (line 18), which means the Monaco Editor module should only be imported when the component actually renders, not during the test's beforeAll hook.
The await import('./index') only imports the registration code and the wrapper component, not Monaco Editor itself. If tests are timing out, the root cause may be elsewhere (e.g., bundler configuration, module resolution issues, or unrelated CI environment issues).
Consider investigating the actual cause of the timeout before committing to a 30-second timeout. A few alternatives:
- Check if Monaco Editor is actually being imported during beforeAll (add logging or profiling)
- Review the Vitest configuration for any module transformation issues
- If Monaco really does load at import time despite React.lazy, consider mocking the Monaco Editor module in tests since the tests only verify ComponentRegistry registration, not editor functionality
| beforeAll(async () => { | ||
| await import('./index'); | ||
| }); | ||
| }, 30000); // 30 second timeout for Monaco Editor initialization |
There was a problem hiding this comment.
The test suite only verifies ComponentRegistry registration and metadata, but doesn't test any actual Monaco Editor functionality. If Monaco Editor is indeed causing timeout issues (which seems unlikely given the lazy loading architecture), consider mocking the @monaco-editor/react module for these unit tests.
This would:
- Eliminate any potential Monaco-related timeouts
- Make tests run faster
- Keep tests focused on what they're actually testing (registration logic)
- Prevent CI flakiness
Example approach: Use vi.mock('@monaco-editor/react') to provide a stub implementation for tests that don't need the actual editor.
The
@object-ui/plugin-editortest suite was timing out during thebeforeAllhook that imports Monaco Editor. The default Vitest hook timeout (10s) is insufficient for Monaco's initialization in the test environment.Changes
beforeAllhook timeout to 30s inpackages/plugin-editor/src/index.test.tsMonaco Editor is lazy-loaded in the component via
React.lazy()but still loads synchronously when imported during test setup. The timeout accommodates this heavy dependency initialization.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.