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

feat(ct): angular component testing #27783

Closed
wants to merge 15 commits into from

Conversation

sand4rt
Copy link
Collaborator

@sand4rt sand4rt commented Oct 24, 2023

closes: #14153 and https://github.com/sand4rt/playwright-ct-angular

TODO

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@pavelfeldman
Copy link
Member

What's out plan here, is it ready to land? Is it based on Younes's work or is this something different?

Copy link

@yjaaidi yjaaidi left a comment

Choose a reason for hiding this comment

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

Great job @sand4rt!
Here are a couple of things that would be nice to solve before merging.
Let me know if you want me to contribute to your branch directly with some PRs.

The main discussion here is about vite plugin replacement and configuration.

packages/playwright-ct-angular/registerSource.mjs Outdated Show resolved Hide resolved
packages/playwright-ct-angular/registerSource.mjs Outdated Show resolved Hide resolved
packages/playwright-ct-angular/index.js Outdated Show resolved Hide resolved
packages/playwright-ct-angular/package.json Outdated Show resolved Hide resolved
@yjaaidi
Copy link

yjaaidi commented Oct 25, 2023

What's out plan here, is it ready to land?

Hey @pavelfeldman! I just shared my feedback with @sand4rt.
Once we agree on that and fix what can be fixed, the PR should be ready.

Is it based on Younes's work or is this something different?

It doesn't matter anymore. There was just a misunderstanding.
We were just a bit disappointed for not merging efforts but now we are working together to provide what the community is waiting for and that's what matters.

@sand4rt
Copy link
Collaborator Author

sand4rt commented Oct 27, 2023

Hey guys, thanks for the comments! I'm AFK for a couple of days. Will hopefully look at the PR by the end of next week as well

Copy link

@edbzn edbzn left a comment

Choose a reason for hiding this comment

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

The added logo is the Angular.js one, here's the link to the correct Angular logo.

packages/playwright-ct-angular/index.js Outdated Show resolved Hide resolved
@sand4rt
Copy link
Collaborator Author

sand4rt commented Nov 5, 2023

The added logo is the Angular.js one, here's the link to the correct Angular logo.

Hi @edbzn, thanks for the input! Would you like to make the change by creating a PR against this branch? See #27783 (comment) for more details

edit: Angular just released their new logo: https://angular.dev/press-kit

This comment has been minimized.

@yjaaidi
Copy link

yjaaidi commented Nov 13, 2023

I just noticed that the tests themselves are using Angular 15.
Should we create a different folder for Angular 16 & Angular 17 and that would require lots of duplication?
Or should we do something at the test-all.spec.js level but that would be a bit too magical?
Or should the "experimental" CT only handle the last version of the framework?
cc. @mxschmitt @pavelfeldman @sand4rt

@yjaaidi
Copy link

yjaaidi commented Nov 21, 2023

Hey @sand4rt, I just created a create-playwright PR microsoft/create-playwright#106 to move the Angular vite plugin config to "user-land".

This way, we can update the todo list as this:

  • Do not handle routing and let users use router testing harness
  • Update tests to use Angular 17
  • Update the documentation
  • Remove @analogjs/vite-plugin-angular dependency & setup.
  • Update https://github.com/microsoft/create-playwright to generate playwright config with @analogjs/vite-plugin-angular and add it to project dependencies.

Here are the advantages of moving the Angular vite plugin setup to the create-playwright generator:

  • @analogjs/vite-plugin-angular has to catch up with Angular internals to make it work for future versions. Currently, the Angular internals it relies on can break at any Angular minor or patch version. Meaning that @playwright/experimental-ct-angular will have to catch up to by updating the dependency which also means a new Playwright release.
  • Users can easily choose the @analogjs/vite-plugin-angular version of their choice.
  • Users can easily choose the @analogjs/vite-plugin-angular configuration of their choice.
  • Users can easily choose another plugin.
  • This doesn't add any dependency to this workspace.

The drawbacks are:

  • It doesn't work out of the box if users don't use the generator or if they use the package manually without reading the docs.
  • Users will have to manually keep @analogjs/vite-plugin-angular up-to-date.

Of course, this is just a temporary solution until the Angular team releases an official vite plugin which will at least warranty major version compatibility.

@yjaaidi
Copy link

yjaaidi commented Feb 15, 2024

Hey @sand4rt are you available in the upcoming days or weeks so we can finish this 😊
Maybe if you could just update your branch I could send you a PR.
Thanks!

@flobacher
Copy link

Great news @yjaaidi and @sand4rt! Thank you so much for your effort time and persistence!!!

@sand4rt
Copy link
Collaborator Author

sand4rt commented Feb 22, 2024

Maybe if you could just update your branch I could send you a PR.

Hey @yjaaidi, #29544 needs to be resolved before i can update. I could resolve the merge conflicts later on if needed, so don't let that hold you back.

For the people willing to contribute; beta testing would also help a lot to ensure everything operates as expected, your feedback is very welcome!

@yjaaidi
Copy link

yjaaidi commented Feb 23, 2024

Thanks @sand4rt!
@edbzn and I had to update and fix #29544 before moving forward.

I think that we have everything now 😊:

  • ✨ it works with thew new transformation
  • ✨ template rendering (thanks to the new transformation system, amazing job @pavelfeldman)
  • ✨ signal inputs support
  • ✨ moved out analog vite plugin from the package
  • ✨ Angular 17 support
  • ✨ and passing providers

Cf. sand4rt#5

@maartentibau
Copy link

Great great great work! Thank you!!
At this moment this will only support standalone components right?

@zargham-leanix
Copy link

Great work @sand4rt @yjaaidi
When this PR is expected to be merged ?

Copy link

@chronospatian chronospatian left a comment

Choose a reason for hiding this comment

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

With the locator fix these tests should be updated.

@chronospatian
Copy link

@yjaaidi I've created a PR with fixes for the above and support for template strings sand4rt#6

Copy link
Contributor

Test results for "tests 1"

1 flaky ⚠️ [firefox-page] › page/frame-goto.spec.ts:46:3 › should continue after client redirect

27343 passed, 662 skipped
✔️✔️✔️

Merge workflow run.

Copy link
Contributor

Test results for "tests 1"

2 failed
❌ [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable
❌ [installation tests] › playwright-electron-should-work.spec.ts:20:5 › electron should work

2 flaky ⚠️ [chromium-library] › library/capabilities.spec.ts:141:3 › should not crash on showDirectoryPicker
⚠️ [playwright-test] › ui-mode-test-screencast.spec.ts:21:5 › should show screenshots

27009 passed, 610 skipped
✔️✔️✔️

Merge workflow run.

@chronospatian
Copy link

I get errors when I try to import other values from a component file.

// @/components.counter.component
import { Component, EventEmitter, Input, Output } from '@angular/core';

// adding this line
export const count = 10

@Component({
   template: `{{count}}`
})
export class CounterComponent {
   count = count
}
import { test, expect } from '@playwright/experimental-ct-angular';
import { count, CounterComponent } from '@/components/counter.component';

test('should mount', async ({ mount }) => {
  const component = await mount(CounterComponent);

  await expect(component).toHaveText(`${count}`)
});

When I try to run this test I get the following error

SyntaxError: Cannot use import statement outside a module

   at ../src/components/counter.component.ts:1

> 1 | import { Component, EventEmitter, Input, Output } from '@angular/core';
    | ^
  2 |
  3 | @Component({
  4 |   standalone: true,

    at Object.<anonymous> (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:1:1)
    at Object.<anonymous> (/Git/playwright/tests/components/ct-angular/tests/update.spec.ts:2:1)

Error: No tests found.
Make sure that arguments are regular expressions matching test files.
You may need to escape symbols like "$" or "*" and quote the arguments.

If I set "type: module" in package.json, then I get a different error.

Error: Standard Angular field decorators are not supported in JIT mode.

    at PropDecorator (/Git/playwright/tests/packages/core/src/util/decorators.ts:161:17)
    at applyDec (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:1813)
    at/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:3514
    at _applyDecs (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:3685)
    at /Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:23:2

Lastly, if I change the test slightly:

export const count = { value: 10 }

I get yet another error:

SyntaxError: Identifier 'CounterComponent' has already been declared

This is very confusing! I guess this is a limitation of the playwright component test, so the solution here would be to extract the variable to another file that has no Angular imports or component usages, or to not import those values into the test to begin with and use hard coded values in the test.

@yjaaidi
Copy link

yjaaidi commented Jun 3, 2024

I get errors when I try to import other values from a component file.

// @/components.counter.component
import { Component, EventEmitter, Input, Output } from '@angular/core';

// adding this line
export const count = 10

@Component({
   template: `{{count}}`
})
export class CounterComponent {
   count = count
}
import { test, expect } from '@playwright/experimental-ct-angular';
import { count, CounterComponent } from '@/components/counter.component';

test('should mount', async ({ mount }) => {
  const component = await mount(CounterComponent);

  await expect(component).toHaveText(`${count}`)
});

When I try to run this test I get the following error

SyntaxError: Cannot use import statement outside a module

   at ../src/components/counter.component.ts:1

> 1 | import { Component, EventEmitter, Input, Output } from '@angular/core';
    | ^
  2 |
  3 | @Component({
  4 |   standalone: true,

    at Object.<anonymous> (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:1:1)
    at Object.<anonymous> (/Git/playwright/tests/components/ct-angular/tests/update.spec.ts:2:1)

Error: No tests found.
Make sure that arguments are regular expressions matching test files.
You may need to escape symbols like "$" or "*" and quote the arguments.

If I set "type: module" in package.json, then I get a different error.

Error: Standard Angular field decorators are not supported in JIT mode.

    at PropDecorator (/Git/playwright/tests/packages/core/src/util/decorators.ts:161:17)
    at applyDec (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:1813)
    at/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:3514
    at _applyDecs (/Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:3:3685)
    at /Git/playwright/tests/components/ct-angular/src/components/counter.component.ts:23:2

Lastly, if I change the test slightly:

export const count = { value: 10 }

I get yet another error:

SyntaxError: Identifier 'CounterComponent' has already been declared

This is very confusing! I guess this is a limitation of the playwright component test, so the solution here would be to extract the variable to another file that has no Angular imports or component usages, or to not import those values into the test to begin with and use hard coded values in the test.

Hi @chronospatian, could you please provide a repro?
Indeed, type: module (or renaming the test to .mts) is the way to go.
Did you try enabling experimentalDecorators in the playwright's tsconfig?

@chronospatian
Copy link

chronospatian commented Jun 4, 2024

@yjaaidi Here's the reproduction: https://github.com/chronospatian/playwright/blob/debug-component-usages/tests/components/ct-angular/tests/update.spec.ts

I found a partial workaround. The test passes if I keep component usages and non-component usages in separate imports. I can even hold a reference to the component class itself if I import it a second time under an import alias. Neat!

It would be nice if the typescript transform could discriminate between component and non-component usages in the same import statement.

The test will always fail if the spec file tries to import any values from a file that contains references an Angular component with field decorators (such as @Input(). Haven't found a workaround for this yet.

Edit: Setting experimentalDecorators did not fix. I'm pretty sure you can't change Playwright's tsconfig except for baseUrl and paths.

this is preparatory work for allowing zoneless testing
Copy link
Contributor

github-actions bot commented Jun 5, 2024

Test results for "tests 1"

15 passed
✔️✔️✔️

Merge workflow run.

@sand4rt
Copy link
Collaborator Author

sand4rt commented Jul 4, 2024

@dgozman we are currently waiting for a response to resolve the following comments:

  1. feat(ct): angular component testing #27783 (comment)
  2. feat(ct): angular component testing #27783 (comment)
  3. feat(ct): angular component testing #27783 (comment)
  4. feat(ct): angular component testing #27783 (comment)
  5. feat(ct): angular component testing #27783 (comment)
  6. feat(ct): angular component testing #27783 (comment)
  7. feat(ct): angular component testing #27783 (comment)

We also have to take care of the following:

  1. Repair the tests that broke in cc7e9b8
  2. feat(ct): angular component testing #27783 (comment)

@zargham-leanix
Copy link

@sand4rt @yjaaidi @dgozman Could I contribute somehow to move this forward ?

@sand4rt sand4rt requested a review from dgozman August 19, 2024 16:56
@yjaaidi
Copy link

yjaaidi commented Aug 22, 2024

Hi @zargham-leanix,
There’s not much to do.
The PR is ready. We’re just waiting for the team’s approval/feedback, then we can update the PR.

i think the team is busy.

meanwhile, you can use our community support
https://github.com/jscutlery/devkit/tree/main/packages/playwright-ct-angular

@flobacher
Copy link

@sand4rt @dgozman do you maybe have an estimate on when this could land, or what is still missing? A short update would be greatly appreciated. Thank you very much in advance 🙏

@pavelfeldman
Copy link
Member

We are trying to make a responsible decision we would be able to stand by in the long run. Looking at the current component testing trends (https://npmtrends.com/@playwright/experimental-ct-react-vs-@playwright/experimental-ct-svelte-vs-@playwright/experimental-ct-vue), we already have reservations around the vue and svelte support viability. Hybrid frameworks are somewhat compromising the whole idea of component testing we well, which also does not make committing any easier. So please let us take time to figure out where we want to bring the component testing going forward. We would hate to add this support to Playwright only to remove it later.

@pavelfeldman
Copy link
Member

I'll close this PR as the decision is not in the realm of the code, so that we could go back to it when we are ready.

@maartentibau
Copy link

maartentibau commented Oct 9, 2024

With all the effort done by @sand4rt @yjaaidi and so many others, I really hope this functionality gets added to Playwright asap.

I do understand the concerns and it is very important that those are being addressed, but while this feature has been available in Cypress for a long time now, Playwright really can not stay behind if you ask me. This it an essential part of a testing strategy which cannot be ignored.

@pfteter
Copy link

pfteter commented Oct 10, 2024

Hi @pavelfeldman, I don't think the stats are fine.

If you combine the stats for cypress and PW for react
https://www.npmjs.com/package/@cypress/react
@playwright/experimental-ct-react

you are at 200K weekly downloads. To that you can also add JEST/Testing Library mount component tests to have a good feeling how big the market is.

All Enterprise companies I know are now migrating from Cy to PW because of the recent Cy decisions.
Component Testing as part of Unit testing is a "new" word for something that has been around for a while and still not adopted in the wild.

What is the alternative JEST/ Testing Library ?
you cant write E2E tests for components you need the mount / test with different parameters

  • Remove the experimental tag, add some documentation
  • Do some marketing

Then make a decision.

I think PW / CT is by far the best technology and I used JEST/Tesing Lirbary / Cy Component Tests. I wrote my own PW CT for Web Components and it works amazingly well - the best thing there is a tolerable tradeoff compared with JEST regarding performance. but you get all the cool tools and stability that make it a amazing developer experience.

@Tallyb
Copy link

Tallyb commented Oct 10, 2024

My 2c: (also answering @pfteter)
Testing components in a browser is a must!
We went with Storybook for rendering the components, and running PW tests on top of it. For us, it is the best of both worlds: SB for rendering components in isolation (8 years of experience in this specific domain) and PW for testing and browser automation.

@yjaaidi
Copy link

yjaaidi commented Oct 11, 2024

We, JSCutlery, will keep maintaining @jscutlery/playwright-ct-angular.

In case @playwright/experimental-ct-core diverges too much and we can't rely on the shared building blocks, we are ready to keep a similar API while maintaining our own.
A prerequisite to keep the same developer experience is to still be able to control babel plugins. @pavelfeldman is it too soon to tell if this will still be exposed?

Would it make sense to move @jscutlery/playwright-ct-angular to https://github.com/playwright-community?

@mxschmitt
Copy link
Member

@yjaaidi I invited you to the org, happy to have it there until Playwright's CT is ready.

@sand4rt
Copy link
Collaborator Author

sand4rt commented Oct 14, 2024

@yjaaidi I invited you to the org, happy to have it there until Playwright's CT is ready.

@mxschmitt any change this PR could be moved to https://github.com/playwright-community?

@JessicaSachs
Copy link

JessicaSachs commented Oct 15, 2024

I wanted to take a moment to respond here, as this PR touches on an issue that’s been critical to the community: the future of browser-based component testing in Playwright.

After four years, the community is still waiting for a decision on whether Playwright will fully commit to component testing. I understand the need for careful, long-term decision-making, but the uncertainty is holding back progress. The concerns about supporting Vue, Svelte, and other frameworks would be valid if the metrics being used to evaluate them captured the full demand. @pavelfeldman You're solely citing experimental download counts for a product you do not teach or document. C'mon man.

As the former product and technical lead for Cypress Component Testing, I’ve had a front-row seat to the innovation and development of the browser-based testing space. Since leaving Cypress, I've been involved in the development of Vitest and its browser modes. Throughout that time I've consistently heard from developers across frameworks about the demand for browser-based component testing. The tools they use—Testing Library, Storybook, Vue Test Utils, and Enzyme—show just how important this feature is to the broader community.

Screenshot 2024-10-14 at 6 28 13 PM

Every time I teach a workshop, I hear the same question: “When will Playwright component testing go stable?” Recently, at VueConf US, a team lead from General Electric expressed their frustration at having to wait for a feature that they already had with Cypress before migrating to Playwright. The demand is clear—developers are ready for Playwright to catch up, but the lack of a stable component testing feature is frustrating.

This PR is especially significant because critical Angular community and core team members spent a year working on and waiting for this effort to be completed. Angular has always been at the forefront of JavaScript testing, and their contribution here reflects how important this feature is. If Playwright isn’t going to move forward with first-party support for component testing, the community deserves to know, so we can focus on building the tools that will meet our needs.

There's need to see code in a browser and to be able to dispatch real events to the side effects (renders/events) created in the DOM. Playwright is currently the BEST and MOST COMPOSABLE tool to create that browser environment and dispatch those actions. I'm betting that Microsoft's priorities and staffing may not align with the community's needs. Which is fine, but leads me to ask for closure. We can read the room, but it would help everyone out a lot in their respective technical discussions and the way people choose to spend their time if the team explicitly signaled their intent for the feature.

Whether Playwright decides to fully commit to component testing or officially deprecate it, the important thing is giving developers the clarity they need to move forward. We’ve waited for four years, and developers need to know where to invest their time and resources.

I’m rooting for progress, whether that’s through Playwright or by empowering the community to build new tooling. Thank you for your time and consideration, and for all the work that’s gone into this so far.

edit: image size

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.

[Feature] Support for Component Tests in Angular apps