Skip to content
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

Migrate to Cypress 10 #74

Merged
merged 13 commits into from
Nov 16, 2022
Merged

Migrate to Cypress 10 #74

merged 13 commits into from
Nov 16, 2022

Conversation

iamdharmesh
Copy link
Member

@iamdharmesh iamdharmesh commented Nov 2, 2022

Description of the Change

This PR upgrades the Cypress version to 10 and updates needed files as part of the migration.

Closes #67

How to test the Change

Make sure PR PASS E2E tests check OR verify E2E tests locally.

Changelog Entry

Changed: Migrated to Cypress 10

Credits

Props @iamdharmesh @cadic

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@iamdharmesh iamdharmesh self-assigned this Nov 2, 2022
@iamdharmesh iamdharmesh added this to the Future Release milestone Nov 2, 2022
@iamdharmesh iamdharmesh marked this pull request as ready for review November 2, 2022 13:47
@iamdharmesh iamdharmesh requested review from a team and peterwilsoncc and removed request for a team November 2, 2022 13:48
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a couple of notes inline, one of which is a question.

@@ -16,8 +16,8 @@
"prepare": "husky install",
"prettify": "prettier --write \"**/*.{ts,js}\"",
"typecheck": "tsc --noEmit",
"cypress:open": "cypress open --config-file tests/cypress/config.json",
"cypress:run": "cypress run --config-file tests/cypress/config.json",
"cypress:open": "cypress open --config-file tests/cypress/cypress-config.js --e2e --browser chrome",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use a custom script that allows the --browser parameter to be overridden when running the script?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterwilsoncc The --browser parameter was added here to the auto-select browser during the test, otherwise, we have to select the browser from UI during the run test in open mode.

Also, we can use a script like this npm run cypress:open -- --browser=firefox to select the Firefox browser or npm run cypress:open -- --browser=electron for Electron.

Should I remove the --browser parameter?

Comment on lines +88 to +98
cy.get('body').then($body => {
let name = 'Status & visibility';
if (
$body.find(
'.components-panel__body .components-panel__body-title button:contains("Summary")'
).length > 0
) {
name = 'Summary';
}
cy.openDocumentSettingsPanel(name);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selector for the component hasn't changed, ie it is .components-panel .edit-post-post-status in both 6.0 and 6.1. Is it possible to use that instead?

Otherwise, maybe see if it's possible to catch the change in openDocumentSettingsPanel to maintain backward compatibility for other repos using the function. @cadic may have some thoughts here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterwilsoncc the cy.openDocumentSettingsPanel() command is using the title as identifier of the panel because in general most of panels doesn't have a unique class :(

Screenshot 2022-11-09 at 15 37 29

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it is better to catch the change in openDocumentSettingsPanel to get this maintained among other repos as well. what do you think @cadic @Sidsector9?

@cadic cadic self-requested a review November 16, 2022 11:06
Copy link
Contributor

@cadic cadic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamdharmesh could you please resolve conflicts?

Nevermind, did that myself

@cadic cadic merged commit f716bda into trunk Nov 16, 2022
@cadic cadic deleted the enhancement/67 branch November 16, 2022 12:18
github-actions bot pushed a commit that referenced this pull request Nov 16, 2022
* Upgrade NPM packages.

* Upgrade config file.

* Renamed test folder

* Removed plugins

* upgrade support file

* Update npm scripts.

* Test folder path update.

* downgrade typedoc package to keep doc site as it is.

* Stop video to check if it can make e2e tests fast.

* Fix test failing in 6.1

* Add dependencies back

* Fix tests.

Co-authored-by: Max Lyuchin <maxim.lyuchin@gmail.com>
@jeffpaul jeffpaul modified the milestones: Future Release, 0.1.0 Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to Cypress 10
4 participants