Conversation
|
There was a problem hiding this comment.
Code Review
This pull request migrates the @firebase/app-check package's test suite from Karma, Mocha, Chai, and Sinon to Vitest. This involves updating configuration files, package scripts, and test files to use Vitest's APIs and assertions. The review feedback points out several remaining instances of legacy Chai .to.equal assertions in internal-api.test.ts that should be updated to Vitest's .toBe() matcher.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the app-check package's test suite from Karma, Mocha, Chai, and Sinon to Vitest, updating configuration files, test scripts, and test assertions accordingly. It also refactors some type-only exports to use export type. The review feedback identifies several opportunities to improve test isolation and robustness: specifically, ensuring mock wrappers for readDebugTokenFromIndexedDB correctly forward arguments to the underlying implementation, and properly cleaning up global variables like self.FIREBASE_APPCHECK_DEBUG_TOKEN and self.grecaptcha in afterEach blocks to prevent test pollution.
Summary
Migrates
@firebase/app-checkunit tests from legacy Karma (Mocha / Chai / Sinon) to Vitest (Browser Chromium via Playwright).Description
karma.conf.jswithvitest.config.mjsextendingconfig/vitest.base.mjs. Configured browser runner targeting Chromium via Playwright with defensive project filtering (if (config.test?.projects)). Updatedtest/setup.tswith globalafterEachteardown (vi.useRealTimers(),vi.resetAllMocks(),vi.restoreAllMocks()).@firebase/app-checkrelies on browser APIs (self.grecaptcha,IndexedDB,localStorage, DOM<script>injection,document.hasFocus, page visibility/proactive refresh timers). All 8 unit test suites run in thebrowserproject via@vitest/browser-playwright.expect(...).to.equal,.to.deep.equal,.to.be.true,.to.be.rejectedWith,.to.throw) to native Vitest matchers (.toBe(),.toEqual(),.toBe(true),.rejects.toThrow(),.toThrow()). Converted Mocha'sdonecallback insrc/internal-api.test.tsto Promise-based async execution. StandardizedafterEachcleanup for globals (self.grecaptcha,self.FIREBASE_APPCHECK_DEBUG_TOKEN).sinon.stub,sinon.spy,sinon.useFakeTimers) to native Vitest utilities (vi.fn(),vi.spyOn(),vi.useFakeTimers()). Usedvi.mock('...', { spy: true })inapi.test.ts,debug.test.ts,internal-api.test.ts,providers.test.ts, andstorage.test.tsso sealed ESM module exports (./client,./indexeddb,./storage,./recaptcha,./debug,./util,./internal-api,@firebase/util) can be spied and overridden per test viavi.spyOn(...)while keeping their original implementations active by default.declare module '@firebase/component'todeclare module '@firebase/component/dist/src/types'insrc/index.tsandsrc/public-types.tsso Vite's ESM resolver properly mergesNameServiceMappingfor'app-check'and'app-check-internal'.package.jsonscripts (test,test:ci,test:browser,test:browser:debug) to run Vitest and removed legacy Karma scripts (test:karma,test:karma:debug).Performance