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.

Sign up for GitHub

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

Jump to bottom

Remove Cypress #52905

Open
21 of 61 tasks
huyenltnguyen opened this issue Jan 3, 2024 · 2 comments
Open
21 of 61 tasks

Remove Cypress #52905

huyenltnguyen opened this issue Jan 3, 2024 · 2 comments
Labels
scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. status: PR in works Work in Progress (WIP) Issues.

Comments

@huyenltnguyen
Copy link
Member

huyenltnguyen commented Jan 3, 2024 •
edited

Description

This issue to track the work needed for the Cypress removal.

Summary

We have added a significant amount of Playwright tests to the codebase, so I think we're in a good position to start removing Cypress.

I'd propose we proceed by going through each Cypress file and check if the test cases are written in Playwright. Add the tests in if they aren't.

Potential impediments

There is a significant discrepancy I've found and I'm afraid it might complicate the migration work. It's that Cypress has a .task() API that allows us to execute a Node command while Playwright doesn't. With this API, we can choose to work with new user data or certified user data in each test case, but we don't have this flexibility with Playwright.

IMHO, the ability to write tests with new user data is important, as we need to have test coverage for flows such as completing a new step/challenge, claiming a certifications, etc.

A couple approaches I have so far:

  1. Keep using certified user data, but mock APIs when needed, either manually or using HAR files.
    1.1 Mocking manually:

    freeCodeCamp/e2e/email-sign-up.spec.ts

    Lines 113 to 128 in 0d123e0

    await page.route('*/**/user/get-session-user', async route => {
    const response = await route.fetch();
    const json = await response.json();
    // /email-sign-up is only accessible if `acceptedPrivacyTerms` is `false`.
    // We need to patch the response in order to access the page.
    json.user.certifieduser.acceptedPrivacyTerms = false;
    json.user.certifieduser.sendQuincyEmail = false;
    await route.fulfill({ json });
    });
    // Intercept the endpoint to prevent `acceptedPrivacyTerms` from being set
    await page.route('*/**/update-privacy-terms', async route => {
    const json = [{ message: 'flash.privacy-updated', type: 'success' }];
    await route.fulfill({ json });
    });

    1.2 Using HAR files: https://playwright.dev/docs/mock#mocking-with-har-files
  2. Run exec() within Playwright tests when needed. Example: https://stackoverflow.com/questions/71397181/node-execute-command-asynchronously-and-read-results-on-completion
  3. Split the Playwright CI flow into one that runs with seed and one that runs with seed:certified-user

I'm still unsure which approach I prefer:

  • (1) is okay, but we lose some confidence as the APIs aren't tested
  • (2) should work, but I have never executed a Node command directly in tests, so I'm not sure if there will be any problems coming from this
  • (3) is probably the ideal way to setup and execute our tests. But the changes will be quite involved as we will need to specify which test suites should be run by which CI flow

Action items

Setup a GHA workflow that runs Playwright test with pnpm run seed

  • Re-organize the e2e directory to facilitate the CI change
  • Setup a GHA workflow that runs Playwright test with pnpm run seed

Add missing Playwright test cases

Migrate and remove Cypress test files

Remove Cypress

  • Remove Cypress commands
  • Remove the entire cypress directory
  • Remove Cypress from the dependency list
  • Remove Cypress GHA workflows

Dev notes

Development notes
  • cypress/e2e/default/learn/responsive-web-design/show-cert-from-superblock.ts
    • Can be removed as the purpose of the tests is to check if the user is able to view their certifications on the settings page. We already have this case covered by the following Playwright test cases:
    • However, the tests include simulating the claim cert workflow, for which we don't have corresponding test cases in Playwright
    • New tests should be added for this workflow, and ideally with the user data from seed instead of seed:certified-user
  • cypress/e2e/default/learn/challenges/projects.ts
  • cypress/e2e/default/settings/certifications.ts
    • The tests simulate the claim certification workflow with the following scenarios:
      • When the user has not agreed to the academic honesty policy
      • When the user has agreed to the academic honesty policy
    • However, the tests aren't reflecting the actual UX correctly as what the assertions don't match what shown on the screen. This issue can be reproduced by manually testing the workflow and compare the UX/UI with the test expectations. I'm not sure how the tests have been passing, TBH.
    • New tests should be added for this workflow, and ideally with the user data from seed instead of seed:certified-user.
@huyenltnguyen huyenltnguyen added status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. labels Jan 3, 2024
@freeCodeCamp freeCodeCamp locked and limited conversation to collaborators Jan 3, 2024
@ShaunSHamilton
Copy link
Member

Thanks for tracking this.

1: No thank you 🙂
2: This should work, provided:
a) Tests are not run in parallel
b) The order of the tests is predictable (or, we have to seed and mock for every test)
3: Seems like the ideal solution, and, provided we can specify a directory to run for playwrite (i know we can), then it is more like having two e2e folders, and should not require much more work than answering a quick question: "Does this test require the mocked Camper or new Camper"

For 3, we might even have 3 directories:

  1. e2e/new/
  2. e2e/mocked/
  3. e2e/independent

@Sembauke Sembauke pinned this issue Jan 9, 2024
@ojeytonwilliams
Copy link
Contributor

I agree that 3) is the way to go for now.

In the future we can seed multiple users and login to whichever user is appropriate for the test, but that's a little way off.

@Sembauke Sembauke unpinned this issue Jan 10, 2024
@Sembauke Sembauke pinned this issue May 2, 2024
@huyenltnguyen huyenltnguyen added status: PR in works Work in Progress (WIP) Issues. and removed status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. labels May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. status: PR in works Work in Progress (WIP) Issues.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ojeytonwilliams @huyenltnguyen @ShaunSHamilton