-
Notifications
You must be signed in to change notification settings - Fork 83
[FSSDK-12089] run tests in browser using vitest #1125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR configures browser-based unit testing using Vitest, enabling tests to run on both local browsers and BrowserStack with support for multiple browser types (Chrome, Firefox, Edge, Safari). A new CI workflow job has been added to automate browser testing.
- Added comprehensive Vitest browser configuration with BrowserStack integration and local browser fallback
- Implemented custom Vitest plugins for request logging and console capture to improve debugging capabilities
- Updated test mocks to be compatible with browser execution environment and refactored mock patterns for better compatibility
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.browser.config.mts | New browser test configuration with BrowserStack and local browser support |
| vitest.config.mts | Updated test file pattern to be more specific |
| vitest/request-logger-plugin.ts | Custom plugin for logging HTTP requests/responses during browser tests |
| vitest/console-capture-plugin.ts | Custom plugin for capturing and forwarding browser console logs to the test runner |
| public/console-capture.js | Client-side script for capturing browser console output and sending to server |
| scripts/run-browser-tests.js | Main orchestration script for running tests across different browser configurations |
| scripts/patch-vitest-viewport.js | Script to patch Vitest viewport commands for browser compatibility |
| lib/utils/cache/local_storage_cache.browser.spec.ts | Fixed test to sort keys before comparison for deterministic results |
| lib/project_config/project_config_manager.ts | Added blank line for readability |
| lib/project_config/project_config_manager.spec.ts | Updated test to provide error message to reject() call |
| lib/project_config/config_manager_factory.react_native.spec.ts | Changed async import to sync require for browser compatibility |
| lib/logging/logger_factory.spec.ts | Expanded mock to explicitly define all exported values for browser mode |
| lib/event_processor/event_processor_factory.spec.ts | Updated mocks to provide explicit constructor functions for browser compatibility |
| lib/event_processor/event_processor_factory.react_native.spec.ts | Changed async import to sync require for browser compatibility |
| lib/event_processor/event_processor_factory.browser.spec.ts | Refactored mocks to avoid importOriginal() which doesn't work in browser mode |
| lib/event_processor/batch_event_processor.spec.ts | Fixed mock setup and sorting logic for deterministic test behavior |
| lib/core/decision_service/index.ts | Made bucketer import path explicit with /index suffix |
| lib/core/decision_service/index.spec.ts | Refactored bucketer mock to use spy mode instead of hoisted mock |
| lib/core/bucketer/index.spec.ts | Updated mock pattern to use spy mode with proper MockInstance types |
| lib/core/audience_evaluator/index.spec.ts | Updated mock pattern to use spy mode with proper MockInstance types |
| lib/shared_types.ts | Changed regular imports to type imports for better tree-shaking and clarity |
| package.json | Added new dependencies for browser testing and updated nise version |
| .gitignore | Added .env to ignore list |
| .github/workflows/javascript.yml | Added browser_tests job to CI workflow with matrix strategy for multiple browsers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| OdpManager, OdpSegmentApiManager, | ||
| OdpSegmentManager, OptimizelySegmentOption, RequestHandler |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reformatting of the export statement spreads multiple exports across a single line with inconsistent style. This reduces readability. Consider either keeping one export per line or using a more consistent multi-line format with proper indentation.
| OdpManager, OdpSegmentApiManager, | |
| OdpSegmentManager, OptimizelySegmentOption, RequestHandler | |
| OdpManager, | |
| OdpSegmentApiManager, | |
| OdpSegmentManager, | |
| OptimizelySegmentOption, | |
| RequestHandler |
|
|
||
| // This script patches files in node_modules/@vitest/browser, so it depends | ||
| // on particular implementation details in that file. So, the @vitest/browser | ||
| // vesion should be pinned in package.json devDependencies to avoid |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "vesion" should be "version".
| // vesion should be pinned in package.json devDependencies to avoid | |
| // version should be pinned in package.json devDependencies to avoid |
| ); | ||
| }); | ||
|
|
||
| it('should return decision response with variation null when a user is bucketed into a different grouped experiment than the one speicfied', () => { |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "speicfied" should be "specified".
| it('should return decision response with variation null when a user is bucketed into a different grouped experiment than the one speicfied', () => { | |
| it('should return decision response with variation null when a user is bucketed into a different grouped experiment than the one specified', () => { |
| // @ts-ignore | ||
| res.write = function(chunk: any, ..._args: any[]) { | ||
| chunks.push(Buffer.from(chunk)); | ||
| return true; | ||
| }; | ||
|
|
||
| // @ts-ignore | ||
| res.end = function(chunk: any, ...args: any[]) { |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @ts-ignore directives are being used to suppress TypeScript errors when overriding the write and end methods. While this works, consider using proper type assertions or the safer @ts-expect-error directive which will error if the suppression becomes unnecessary in future TypeScript versions.
| // @ts-ignore | ||
| res.write = function(chunk: any, ..._args: any[]) { | ||
| chunks.push(Buffer.from(chunk)); | ||
| return true; | ||
| }; | ||
|
|
||
| // @ts-ignore |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @ts-ignore directives are being used to suppress TypeScript errors when overriding the write and end methods. While this works, consider using proper type assertions or the safer @ts-expect-error directive which will error if the suppression becomes unnecessary in future TypeScript versions.
| // @ts-ignore | |
| res.write = function(chunk: any, ..._args: any[]) { | |
| chunks.push(Buffer.from(chunk)); | |
| return true; | |
| }; | |
| // @ts-ignore | |
| // @ts-expect-error | |
| res.write = function(chunk: any, ..._args: any[]) { | |
| chunks.push(Buffer.from(chunk)); | |
| return true; | |
| }; | |
| // @ts-expect-error |
| } | ||
|
|
||
| // BrowserStack Local is optional - only needed if tests require localhost access | ||
| const useBrowserStackLocal = process.env.BROWSERSTACK_LOCAL === 'true'; |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable useBrowserStackLocal.
| const useBrowserStackLocal = process.env.BROWSERSTACK_LOCAL === 'true'; |
| let bs_local = null; | ||
|
|
||
| function startTunnel() { | ||
| const username = process.env.BROWSERSTACK_USERNAME || process.env.BROWSER_STACK_USERNAME; |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable username.
| const username = process.env.BROWSERSTACK_USERNAME || process.env.BROWSER_STACK_USERNAME; |
Summary
Test plan
Issues