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

added testcases covering all major browsers, removed cypress & added playwright 🧪 #1481

Merged
merged 9 commits into from
Apr 8, 2024

Conversation

shilCode
Copy link
Contributor

@shilCode shilCode commented Mar 23, 2024

First thing, PLEASE READ THIS: ReactPlay Code Review Checklist

Description

Firstof all, love the work all of you are doing! I was trying to learn react and as a QA engineer myself the testing bit caught my eye (obiously) or atleast lack of robustness of it did. So in this pr I tried to write some critcal path e2e tests to contribute atleast something to react-play community. I wanted to write on top of the already present cypress test-cases however soon realized there aren't that much of tests so wanted to give capability of mutil-browser such as firefox and safari - thus added playwright and removed all cypress dependencies. Playwright is much better than cypress, there are millions of documents which will say that but I use both in my day to day work so it doesn't matter to me much but in this case I think playwright suits better. I made sure nothings breaks with this change and all the previous tests are covered as much as possible. In a future pull request I will try to add mobile capability

To view all the changes I made checkout my commit messages, but here are some big changes I made:

  1. Removed Cypress fully
  2. Added Playwright
  3. Wrote 18 testcases per browser for 3 browser

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

enhanced testing capabilities itself attaching html report

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots or example output

playwright-report.zip

- removed cypress (still need to remove folders and update readme.md)
- installed playwright
- wrote first few tests (homepage)
- firefox and chrome added
- completed homepage fully (need to convert to po still)
- create npm/yarn e2e script
- removed cypress directory since all the tests are covered
- converted homepage to pageobject
- added ideaspage and also added po for it
- fixed main readme file to adjust to playwright
- added plays.spec to write more tests later
- removed cypress.yml and added pw yml
- using list reporter instaed of line reporter
- added full sets of tests
Copy link

netlify bot commented Mar 23, 2024

Deploy Preview for reactplayio ready!

Name Link
🔨 Latest commit 1ef19fa
🔍 Latest deploy log https://app.netlify.com/sites/reactplayio/deploys/6612140e6bed7800088ac8a5
😎 Deploy Preview https://deploy-preview-1481--reactplayio.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hey! contributor, thank you for opening a Pull Request 🎉.

@reactplay/maintainers will review your submission soon and give you helpful feedback. If you're interested in continuing your contributions to open source and want to be a part of a welcoming and fantastic community, we invite you to join our ReactPlay Discord Community.
Show your support by starring ⭐ this repository. Thank you and we appreciate your contribution to open source!
Stale Marking : After 30 days of inactivity this issue/PR will be marked as stale issue/PR and it will be closed and locked in 7 days if no further activity occurs.

e2e/pageobjects/pages/home.ts Outdated Show resolved Hide resolved
@priyankarpal priyankarpal added the changes required need to change some thing label Mar 26, 2024
mohatShilABB and others added 2 commits March 26, 2024 14:52
Signed-off-by: Mohat Shil <120472338+shilCode@users.noreply.github.com>
Signed-off-by: Mohat Shil <120472338+shilCode@users.noreply.github.com>
@priyankarpal
Copy link
Member

will check soon

@priyankarpal priyankarpal added ✨ goal: improvement Improvement to an existing feature and removed changes required need to change some thing labels Apr 7, 2024
@priyankarpal
Copy link
Member

there are a few failed test cases & most of them have the same error.

Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:3000/
Call log:
  - navigating to "http://localhost:3000/", waiting until "load"


  4 |
  5 | test.beforeEach(async ({ page }) => {
> 6 |   await page.goto('/');
    |              ^
  7 |   await page.waitForLoadState('domcontentloaded');
  8 | });
  9 |

    at /workspaces/react-play/e2e/tests/homepage.spec.ts:6:14

Screenshot_2024-04-07_09-21-56

@shilCode
Copy link
Contributor Author

shilCode commented Apr 7, 2024

there are a few failed test cases & most of them have the same error.

Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:3000/
Call log:
  - navigating to "http://localhost:3000/", waiting until "load"


  4 |
  5 | test.beforeEach(async ({ page }) => {
> 6 |   await page.goto('/');
    |              ^
  7 |   await page.waitForLoadState('domcontentloaded');
  8 | });
  9 |

    at /workspaces/react-play/e2e/tests/homepage.spec.ts:6:14

Screenshot_2024-04-07_09-21-56

Thanks @priyankarpal for reviewing the pr 🙂
The issue that you are facing could be due to pw cannot find the /ideas path, if you can kindly pass me some more info so I can review it better 🙏and fix the root-cause so that no-one else face it.

  1. I suggest to try to navigate to the url in any browser <localhost>/ideas if that is reachable but while running pnpm e2e or npm run e2e it occurs then its a pw issue. Also note dev envrionment needs to be actively running to test pnpm dev or pnpm start otherwise url configuration is required in playwright.config.ts file > baseURL line
  2. Consider running the ideaspage.spec.ts file in --headed mode: to achieve that use npx playwright test ideaspage.spec.ts --debug --retries=0 --reporter=list,html --worker=1 this will open the test in browser and "playwright inspector", in the inspector use step over button to go through step by step each line. when the tests will fail it will provide a report - which should be at playwright-report folder - if you can zip the folder and provide me that I can debug as well.

so basically if /ideas path is changed in dev environment than pw cannot parse through that, user need to adjust the test goto path manually.

here is a gif version of mentioned steps
ideasTestIssue

you can also use official playwright official extention to repro it in visual way however it might not provide reports properly.

again thanks for looking into this 🙂

@priyankarpal
Copy link
Member

ok ok let me try this

@priyankarpal
Copy link
Member

working 🚀

@priyankarpal priyankarpal merged commit c95c260 into reactplay:main Apr 8, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ goal: improvement Improvement to an existing feature waiting for reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants