-
Notifications
You must be signed in to change notification settings - Fork 437
chore(shared): Migrate tests to vitest #7014
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| --- | ||
| --- |
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,72 @@ | ||||||||||||||||||||||||
| import type { Color } from '@clerk/types'; | ||||||||||||||||||||||||
| import { describe, expect, it } from 'vitest'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { colorToSameTypeString, hexStringToRgbaColor, stringToHslaColor, stringToSameTypeColor } from '../color'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('stringToHslaColor(color)', function () { | ||||||||||||||||||||||||
| const hsla = { h: 195, s: 1, l: 0.5 }; | ||||||||||||||||||||||||
| const cases: Array<[string, Color | null]> = [ | ||||||||||||||||||||||||
| ['', null], | ||||||||||||||||||||||||
| ['transparent', { h: 0, s: 0, l: 0, a: 0 }], | ||||||||||||||||||||||||
| ['#00bfff', hsla], | ||||||||||||||||||||||||
| ['00bfff', hsla], | ||||||||||||||||||||||||
| ['rgb(0, 191, 255)', hsla], | ||||||||||||||||||||||||
| ['rgba(0, 191, 255, 0.3)', { ...hsla, a: 0.3 }], | ||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it.each(cases)('.stringToHslaColor(%s) => %s', (a, expected) => { | ||||||||||||||||||||||||
| expect(stringToHslaColor(a)).toEqual(expected); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('hexStringToRgbaColor(color)', function () { | ||||||||||||||||||||||||
| const cases: Array<[string, Color | null]> = [ | ||||||||||||||||||||||||
| ['#00bfff', { r: 0, g: 191, b: 255 }], | ||||||||||||||||||||||||
| ['00bfff', { r: 0, g: 191, b: 255 }], | ||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it.each(cases)('.hexStringToRgbaColor(%s) => %s', (a, expected) => { | ||||||||||||||||||||||||
| expect(hexStringToRgbaColor(a)).toEqual(expected); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('stringToSameTypeColor(color)', function () { | ||||||||||||||||||||||||
| const cases: Array<[string, Color | null]> = [ | ||||||||||||||||||||||||
| ['', ''], | ||||||||||||||||||||||||
| ['invalid', ''], | ||||||||||||||||||||||||
| ['12ff12', '#12ff12'], | ||||||||||||||||||||||||
| ['#12ff12', '#12ff12'], | ||||||||||||||||||||||||
| ['1ff', '#1ff'], | ||||||||||||||||||||||||
| ['transparent', 'transparent'], | ||||||||||||||||||||||||
| ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }], | ||||||||||||||||||||||||
| ['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }], | ||||||||||||||||||||||||
| ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }], | ||||||||||||||||||||||||
| ['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }], | ||||||||||||||||||||||||
|
Comment on lines
+41
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove duplicate test cases. Lines 43-44 are exact duplicates of lines 41-42 (testing the same rgb/rgba inputs). Remove the redundant entries. Apply this diff: ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
- ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
- ['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
['hsl(244,66%,33%)', { h: 244, s: 0.66, l: 0.33, a: undefined }],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| ['hsl(244,66%,33%)', { h: 244, s: 0.66, l: 0.33, a: undefined }], | ||||||||||||||||||||||||
| ['hsla(244,66%,33%,0.5)', { h: 244, s: 0.66, l: 0.33, a: 0.5 }], | ||||||||||||||||||||||||
| ['hsl(244,66%,33)', ''], | ||||||||||||||||||||||||
| ['hsla(244,66%,33,0.5)', ''], | ||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it.each(cases)('.stringToSameTypeColor(%s) => %s', (a, expected) => { | ||||||||||||||||||||||||
| expect(stringToSameTypeColor(a)).toEqual(expected); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('colorToSameTypeString(color)', function () { | ||||||||||||||||||||||||
| const cases: Array<[Color, string]> = [ | ||||||||||||||||||||||||
| ['', ''], | ||||||||||||||||||||||||
| ['invalid', ''], | ||||||||||||||||||||||||
| ['#12ff12', '#12ff12'], | ||||||||||||||||||||||||
| ['#12ff12', '#12ff12'], | ||||||||||||||||||||||||
| ['#1ff', '#1ff'], | ||||||||||||||||||||||||
|
Comment on lines
+57
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix type annotation and remove duplicate test case. Two issues:
Apply this diff to fix the type annotation and remove the duplicate: - const cases: Array<[Color, string]> = [
+ const cases: Array<[Color | string, string]> = [
['', ''],
['invalid', ''],
['#12ff12', '#12ff12'],
- ['#12ff12', '#12ff12'],
['#1ff', '#1ff'],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| [{ r: 100, g: 100, b: 100, a: undefined }, 'rgb(100,100,100)'], | ||||||||||||||||||||||||
| [{ r: 100, g: 100, b: 100, a: 0.5 }, 'rgba(100,100,100,0.5)'], | ||||||||||||||||||||||||
| [{ h: 100, s: 0.55, l: 0.33, a: undefined }, 'hsl(100,55%,33%)'], | ||||||||||||||||||||||||
| [{ h: 100, s: 1, l: 1, a: 0.5 }, 'hsla(100,100%,100%,0.5)'], | ||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it.each(cases)('.colorToSameTypeString(%s) => %s', (a, expected) => { | ||||||||||||||||||||||||
| expect(colorToSameTypeString(a)).toEqual(expected); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Good defensive coding for JSDOM compatibility.
The code checks if the
webdriverproperty exists onwindow.navigatorbefore defining it. This suggests that Vitest's JSDOM environment may not include this property by default, unlike Jest's environment.However, verify whether this defensive check is necessary. If Vitest consistently lacks this property, consider defining it unconditionally in the global test setup (
vitest.setup.mts) rather than in individual test files to reduce duplication.Run the following script to check if this pattern appears in other test files:
🏁 Script executed:
Length of output: 0
I need to try different search approaches since the regex pattern didn't match. Let me search more broadly:
Length of output: 9467
🏁 Script executed:
Length of output: 39064
🏁 Script executed:
Length of output: 0
Consider extracting the repeated webdriver property setup into a helper function to avoid duplication.
The defensive
webdriverproperty check appears twice in the same file:isValidBrowserdescribe block)isValidBrowserOnlinedescribe block)Since the logic is identical and JSDOM appears to consistently lack this property in both test suites, extract this into a shared setup helper to reduce duplication and improve maintainability:
Then call
setupWebdriverProperty()in eachbeforeEachinstead of repeating the full block.🤖 Prompt for AI Agents