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

⬆️(project) upgrade cypress to version 10.11.0 #23

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SergioSim
Copy link
Collaborator

@SergioSim SergioSim commented Nov 8, 2022

Purpose

We want our project to be up to date with the latest cypress release.

Proposal

  • upgrade cypress version to 10.11.0
  • upgrade Javascript dependencies in package.json/yarn.lock
  • add cypress linter plugin

Includes changes from #22. Blocked until is #22 merged.

When bootstraping this project in the dev branch two additional
directories are created: edx/src and edx/modules.
To ease switching between a feature branch and the dev branch we
choose to ignore the edx/modules directory.
When bootstraping this project in the dev branch two additional
directories are created: edx/src and edx/modules.
To ease switching between a feature branch and the dev branch we
choose to make the linter ignore the edx directory.
To keep the circleci machine up to date.
Copy link
Contributor

@jmaupetit jmaupetit left a comment

Choose a reason for hiding this comment

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

Great!

- run:
name: Run e2e tests
command: make run-edx test
command: make test
Copy link
Contributor

Choose a reason for hiding this comment

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

To ensure edx is running, what about the following instead (along with removing the run-edx target in the previous step)?

Suggested change
command: make test
command: make run-edx && make test

@@ -12,14 +12,20 @@ describe("LMS JS Input Response Problem Interaction Test", () => {
cy.lmsEnroll(true);
// Navigate to the courseware.
cy.visit(sectionUrl);
// Wait for iframe to become interactive.
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(1500);
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no better way than enforcing this wait phase, please explain why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right) There is a better way!
The challenge was that the js-input-problem is in a cross-origin iframe (hosted at https://studio.edx.org/c4x/edX/DemoX/asset/webGLDemo.html), and browsers do not allow JavaScript from one domain to access elements in another domain for security reasons. Luckily Cypress allows us to disable browser security. Updated this part here. However, unfortunately, this only works for chrome-based browsers.

@jmaupetit
Copy link
Contributor

I think review comments applies to #22! Sorry for that. Consider #22 and this PR as accepted.

The iframe in the js_input_response test needs some time to become
interactive. Therefore we disable chromeWebSecurity to give cypress
access to the cross-domain iframe to check whether the iframe is ready
or not.
The drag_and_drop test relies on the problem beeing at it's initial
state.
However, edX preserves the state of the problem once it's submitted.
Therefore we reset the drag_and_drop problem before answering it.
We upgrade cypress to its latest version.
We also upgrade the javascript dependencies used for linting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants