-
Notifications
You must be signed in to change notification settings - Fork 0
[Chore] - Updated version of cypress to 14.5.4 and fixed broken cypress tests a… #921
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
[Chore] - Updated version of cypress to 14.5.4 and fixed broken cypress tests a… #921
Conversation
…nd removed loading of 'tinymce' from layout.tsx to TinyMCEEditor
|
|
||
| return ( | ||
| <html lang={locale} className={font_sans_serif.variable}> | ||
| <head> |
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.
Removed this from because even with the beforeInteractive which loads it early in the page lifecycle, it was still loading on every page, and most pages don't use TinyMCEEditor.
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.
good catch.
| }, [isAuthenticated]); | ||
|
|
||
| const handleLogout = async () => { | ||
| setShowMobileMenu(false); |
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 know this Header is just a temporary filler, but noticed that the mobile menu doesn't close when a user clicks on the "Logout" button in the mobile navigation. This was causing the "logout" cypress test to fail.
| <li role="menuitem"> | ||
| <Button | ||
| className="react-aria-Button secondary" | ||
| data-testid="logoutButtonDesktop" |
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.
Added test ids for easier testing in cypress
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.
interesting. I've never seen data-testId used before
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.
Yea it makes it easier to test specific components.
|
|
||
| import { useEffect, useRef, useState } from 'react'; | ||
| import type { Editor as TinyMCEEditorType } from 'tinymce'; | ||
| import { loadTinymceScript } from '@/utils/loadTinyMCE'; |
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.
Moved loading of tinymce to it's own separate util that is used only when TinyMCEEditor is used on a page.
| describe('Authentication flow tests', () => { | ||
| // Base configuration | ||
| const baseUrl = Cypress.env('BASE_URL') || 'http://localhost:3000'; | ||
| const email = Cypress.env('TEST_USER_EMAIL') || '[email protected]'; |
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.
Fixed tests. This was mostly to see if I could get Cypress tests to work, because previously it stopped working when we updated NextJS version.
Glad to see that we can still get cypress to work for the app. We might consider using this for integration tests.
| "@types/sanitize-html": "2.16.0", | ||
| "brace-expansion": "2.0.2", | ||
| "cypress": "14.4.1", | ||
| "cypress": "14.5.4", |
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.
Was reminded about this update by renovate, but since I wanted to fix some cypress tests, I created my own branch for this.
briri
left a comment
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.
Looks good @jupiter007 Happy to hear that the Cypress tests are working again :)
|
|
||
| return ( | ||
| <html lang={locale} className={font_sans_serif.variable}> | ||
| <head> |
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.
good catch.
| <li role="menuitem"> | ||
| <Button | ||
| className="react-aria-Button secondary" | ||
| data-testid="logoutButtonDesktop" |
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.
interesting. I've never seen data-testId used before
|
Thanks so much for the review Brian. |
Description
layout.tsx, to TinyMCEEditor, so that it will be lazy-loaded only when used.