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

Introduce E2E for end user tests #16857

Open
juri-sinitson opened this issue Nov 26, 2024 · 2 comments
Open

Introduce E2E for end user tests #16857

juri-sinitson opened this issue Nov 26, 2024 · 2 comments
Assignees
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Milestone

Comments

@juri-sinitson
Copy link

juri-sinitson commented Nov 26, 2024

Describe the bug

The Problem and the general solution

The existing tests don't seem to work. It looks like the selectors are outdated and it looks like the wrong tool is being used for E2E tests. I'm willing to make them work piece for piece and I would like to sync this topic with the deciding party.

Proposition

  1. To use jest for non-ui-tests aka unit tests only
  2. To make E2E tests directly in the showcase with e.g. Cypress Playwright, see also this example
  3. Make unit tests large in number, E2E tests fewer in number and manual tests even less in number
  4. Beyond the scope: it might be reasonable to use this concept in the long term to gain more performance

Why?

  1. Unit-Tests are performant in my experience when they are used for non-UI stuff only. It's relatively cheap to cover a whole non-ui method with unit-tests

  2. I can't debug the E2E tests quickly (I estimate I need 5 hours for a test suite instead of 1):
    dynamicdialog.spec.ts is an example, I assume it's applicable to all the existing tests in this branch:
    I've tried to make it work since generally the test makes sense for me at least. Theoretically updating the selectors e.g. by using such abstraction like the data-testid attribute in the template should fix it. But it doesn't, so I end up needing to see the component in the browser to debug the test.
    I think simulating the end user at the showcases page make E2E tests much more genuine. But there is no possibility (not known for me at least) to do this with such unit test tool like jasmine or jest. Such tools like Playwright and Cypress are rather made for this in my opinion.

  3. Unit Tests are cheap, E2E tests are costly and manual tests are even more costly in my experience

  4. Already mentioned above

Environment

Reproducible on the local PC

Reproducer

No response

Angular version

18

PrimeNG version

18

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

20.18.1

Browser(s)

No response

Steps to reproduce the behavior

  1. Switch to this branch, install the deps
  2. npx jest packages/primeng/src/dynamicdialog/dynamicdialog.spec.ts --watch

Expected behavior

  1. Non UI-Parts like services and util functions are tested by unit testing with jest
  2. UI is tested on the showcase page by E2E testing with Cypress Playwright
@juri-sinitson juri-sinitson added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Nov 26, 2024
@juri-sinitson
Copy link
Author

juri-sinitson commented Nov 27, 2024

Update

In my opinion Playwright is rather appropriate for an open source project. Unlike by Cypress all features of Playwright seem to be publicly available.

And here are examples of the payed features of Cypress:

  1. UI Coverage
  2. Cloud
  3. Cypress Accessibility,

So for me personally it looks like Cypress is rather appropriate for an enterprise proprietary project than for an open source one.
I speculate that during a year or two Playwright has those Cypress paid features for free.

@mertsincan
Copy link
Member

Thanks a lot for the detailed description! We plan to work on tests after PrimeNG v18. This will help us.

@mertsincan mertsincan self-assigned this Nov 28, 2024
@mertsincan mertsincan added Status: Pending Review Issue or pull request is being reviewed by Core Team and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Nov 28, 2024
@mertsincan mertsincan added this to the 19.x milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Projects
None yet
Development

No branches or pull requests

2 participants