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
test(playwright): add tests for editor theme selection logic #54481
base: main
Are you sure you want to change the base?
Conversation
9c1abdf
to
017580c
Compare
Ugh. Yet another Webkit test that only fails in CI but not local. |
…nto test/editor-theme
…nto test/editor-theme
a052868
to
d5aeb79
Compare
test.use({ colorScheme: 'dark' }); | ||
|
||
test.describe('If the user is signed in', () => { | ||
test.use({ storageState: 'playwright/.auth/certified-user.json' }); |
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.
We do have storageState
set during the setup phase:
freeCodeCamp/e2e/global-setup.ts
Line 8 in 84a81c8
.storageState({ path: 'playwright/.auth/certified-user.json' }); |
But for some reason that doesn't have any effect, and the user is logged out, which causes some tests to fail. Setting storageState
again within the test suite is the only workaround at the moment, until I can figure out what the issue is.
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.
I was confused by this too. It turns out we do setup storageState
, but we we don't use it by default. So, any tests that require a signed-in user have to manually specify test.use
, like you did here.
Do you think it would be more intuitive to default to using it? I'm leaning towards "yes".
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.
Oh, I see.
Yes, I think it makes sense to make this the default (I guess that's the purpose of the global login step, so that we don't have to log in manually).
…nto test/editor-theme
Checklist:
main
branch of freeCodeCamp.This PR:
Note: While the theme selection logic is fairly simple, the implementation involves
window.matchMedia()
, which is an API that JSDOM doesn't support. It will require heavily mocking thematchMedia
API if we write unit tests, while it's much simpler with e2e as Playwright supports this feature out of the box (https://playwright.dev/docs/api/class-testoptions#test-options-color-scheme).