playwright icon indicating copy to clipboard operation
playwright copied to clipboard

feat(ct): angular component testing

Open sand4rt opened this issue 1 year ago • 48 comments

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

TODO

  • [x] ~~Enable vite-plugin-angular JIT mode: https://github.com/analogjs/analog/pull/374#issuecomment-1536991065~~
  • [x] Fix Sourcemap is likely to be incorrect: a plugin (@analogjs/vite-plugin-angular) errors when transpiling: https://github.com/analogjs/analog/issues/410
  • [x] Restructure NPM (peer)dependencies?: https://github.com/microsoft/playwright/pull/27783#discussion_r1372343294
  • [x] Remove typeof plugin.default check: https://github.com/microsoft/playwright/pull/27783/files#diff-4592ac823d44ec894c518ba459cfab4bd544056a674739fda5fc145cdc596923R28
  • [x] Update the documentation (PR ready: https://github.com/sand4rt/playwright/pull/4)
  • [x] Update https://github.com/microsoft/create-playwright
  • [x] Do not handle routing and let users use router testing harness: https://github.com/microsoft/playwright/pull/27783#discussion_r1372305687
  • [x] Update tests to use Angular 17 ? https://github.com/microsoft/playwright/pull/27783#issuecomment-1809269085
  • [x] Remove @analogjs/vite-plugin-angular and related code and move it to create-playwright ? https://github.com/microsoft/playwright/pull/27783#discussion_r1372342351
  • [x] Update old Angular logo: https://github.com/microsoft/playwright/pull/27783#pullrequestreview-1702918975

sand4rt avatar Oct 24 '23 20:10 sand4rt

Test results for "tests 1"

3 flaky :warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
3 interrupted :warning: [installation tests] › playwright-should-work-with-relative-home-path.spec.ts:19:5 › playwright should work with relative home path
:warning: [playwright-test] › playwright.spec.ts:277:5 › should respect headless in modifiers that run before tests
:warning: [playwright-test] › playwright.trace.spec.ts:173:5 › should not mixup network files between contexts

15799 passed, 308 skipped, 185 did not run :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Oct 24 '23 20:10 github-actions[bot]

Test results for "tests 1"

1 failed :x: [playwright-test] › to-have-screenshot.spec.ts:321:5 › should fail to screenshot an element with infinite animation

7 flaky :warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [firefox] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [firefox] › page/page-goto.spec.ts:431:3 › js redirect overrides url bar navigation
:warning: [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
:warning: [webkit] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [webkit] › page/page-goto.spec.ts:266:3 › should fail when navigating to bad SSL
:warning: [playwright-test] › ui-mode-test-ct.spec.ts:55:5 › should run component tests after editing test

25975 passed, 604 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Oct 24 '23 21:10 github-actions[bot]

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

pavelfeldman avatar Oct 25 '23 17:10 pavelfeldman

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.

yjaaidi avatar Oct 25 '23 21:10 yjaaidi

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

sand4rt avatar Oct 27 '23 10:10 sand4rt

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 https://github.com/microsoft/playwright/pull/27783#discussion_r1382610163 for more details

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

sand4rt avatar Nov 05 '23 20:11 sand4rt

Test results for "tests 1"

2 failed :x: [playwright-test] › reporter-html.spec.ts:705:5 › merged › should use file-browser friendly extensions for buffer attachments based on contentType :x: [installation tests] › playwright-packages-install-behavior.spec.ts:38:7 › @playwright/browser-webkit should work

13 flaky :warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [chromium] › library/capabilities.spec.ts:139:3 › should not crash on showDirectoryPicker
:warning: [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
:warning: [chromium] › components/splitView.spec.tsx:65:5 › drag resize
:warning: [webkit] › library/browsercontext-proxy.spec.ts:167:3 › should use ipv6 proxy
:warning: [webkit] › library/browsercontext-storage-state.spec.ts:51:3 › should set local storage
:warning: [webkit] › library/browsercontext-storage-state.spec.ts:166:3 › should not restore localStorage twice
:warning: [webkit] › library/har.spec.ts:595:3 › should have connection details
:warning: [webkit] › page/page-event-popup.spec.ts:165:3 › should report popup opened from iframes
:warning: [webkit] › page/page-goto.spec.ts:81:3 › should work with Cross-Origin-Opener-Policy
:warning: [playwright-test] › ui-mode-test-ct.spec.ts:55:5 › should run component tests after editing test
:warning: [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live
:warning: [playwright-test] › ui-mode-test-progress.spec.ts:165:5 › should update tracing network live

26054 passed, 609 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Nov 05 '23 21:11 github-actions[bot]

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 avatar Nov 13 '23 22:11 yjaaidi

Hey @sand4rt, I just created a create-playwright PR https://github.com/microsoft/create-playwright/pull/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.
  • [X] 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 avatar Nov 21 '23 16:11 yjaaidi

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!

yjaaidi avatar Feb 15 '24 07:02 yjaaidi

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

flobacher avatar Feb 15 '24 09:02 flobacher

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

Hey @yjaaidi, https://github.com/microsoft/playwright/issues/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!

sand4rt avatar Feb 22 '24 19:02 sand4rt

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. https://github.com/sand4rt/playwright/pull/5

yjaaidi avatar Feb 23 '24 15:02 yjaaidi

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

maartentibau avatar Feb 27 '24 17:02 maartentibau

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

zargham-leanix avatar Mar 20 '24 10:03 zargham-leanix

@maartentibau correct, only standalone components are supported at the moment.

@zargham-leanix the last few open tasks of this PR are being resolved in https://github.com/sand4rt/playwright/pull/5. The progress could be followed over there.

sand4rt avatar Apr 05 '24 14:04 sand4rt

Test results for "tests 1"

1 failed :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable

2 flaky :warning: [webkit-library] › library/browsercontext-device.spec.ts:45:5 › device › should scroll to click
:warning: [webkit-library] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker

24088 passed, 504 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 07 '24 20:05 github-actions[bot]

Test results for "tests 1"

2 flaky :warning: [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live
:warning: [playwright-test] › ui-mode-test-progress.spec.ts:165:5 › should update tracing network live

27315 passed, 661 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 07 '24 21:05 github-actions[bot]

Test results for "tests 1"

1 flaky :warning: [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live

27322 passed, 661 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 09 '24 17:05 github-actions[bot]

Test results for "tests 1"

2 flaky :warning: [webkit-library] › library/browsercontext-clearcookies.spec.ts:146:3 › should remove cookies by name and domain
:warning: [playwright-test] › ui-mode-test-watch.spec.ts:96:5 › should batch watch updates

27321 passed, 661 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 09 '24 18:05 github-actions[bot]

Test results for "tests 1"

1 failed :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable

1 flaky :warning: [chromium-library] › library/trace-viewer.spec.ts:908:1 › should display waitForLoadState even if did not wait for it

27321 passed, 661 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 09 '24 20:05 github-actions[bot]

Test results for "tests 1"

27323 passed, 661 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar May 09 '24 21:05 github-actions[bot]

Hey @pavelfeldman, the PR is finally set for review by the Playwright team! We have deliberately kept the adapter minimal for now and postponed the documentation until more people have tested it and shared their feedback. We would like to gradually make improvements here and there after the merge.

sand4rt avatar May 10 '24 14:05 sand4rt

Congratulations to all of you. That's a milestone in Angular testing history!

Just a small remark: From what I've seen, there is no possibility to provide services, right?

If that's the case, it would be great to add this as the next feature once this branch is merged.

As you know, the vast majority of Angular Components usually depend on Services. If we could mock these services, the number of people who could test on a large scale would increase "exponentially."

Either way, congratulations and a thousand thanks again. I'm looking forward to it

rainerhahnekamp avatar May 11 '24 10:05 rainerhahnekamp

Congratulations to all of you. That's a milestone in Angular testing history!

Just a small remark: From what I've seen, there is no possibility to provide services, right?

If that's the case, it would be great to add this as the next feature once this branch is merged.

As you know, the vast majority of Angular Components usually depend on Services. If we could mock these services, the number of people who could test on a large scale would increase "exponentially."

Either way, congratulations and a thousand thanks again. I'm looking forward to it

Providers/imports are still open for discussion indeed. We've decided to merge the library first before continuing the discussion: https://github.com/sand4rt/playwright/pull/5#discussion_r1541178625

I think it could either be supported like the other frameworks through the hooksConfig:

// playwright/index.ts
beforeMount(async ({ TestBed, hooksConfig }) => {
  TestBed.configureTestingModule({
    imports: hooksConfig?.imports,
    providers: hooksConfig?.providers
  });
});
test('hooks config', async ({ mount }) => {
  const component = await mount(AngularComponent, {
    hooksConfig: {
      imports: [Component],
      providers: [Service],
    }
  });
  await expect(component).toContainText('boop');
});

or with dedicated APIs:

test('imports and providers', async ({ mount }) => {
  const component = await mount(AngularComponent, {
    imports: [Component],
    providers: [Service],
  });
  await expect(component).toContainText('boop');
});

sand4rt avatar May 11 '24 10:05 sand4rt

Understood, then let's wait for the merge and then continue the discussion. Congrats again.

rainerhahnekamp avatar May 11 '24 15:05 rainerhahnekamp

Amazing work @sand4rt @yjaaidi ... this really is a milestone. Can't thank you guys enough for this effort! 👏 👏 👏

maartentibau avatar May 12 '24 12:05 maartentibau

@yjaaidi I am running into some issues while testing this library. ~~https://github.com/sand4rt/playwright-ct-angular/issues/50~~ Resolved ~~https://github.com/sand4rt/playwright-ct-angular/issues/51~~ Resolved ~~https://github.com/sand4rt/playwright-ct-angular/issues/52~~ Resolved

~~I have pulled and linked the PR branch and still face these errors.~~ I also cannot import from @angular/material without adding "type": "module" to my package.json, or changing all my files to .mts.

Other than that it's working great so far. Thanks so much for this.

chronospatian avatar May 13 '24 01:05 chronospatian

@yjaaidi I am running into some issues while testing this library. sand4rt/playwright-ct-angular#50 ~sand4rt/playwright-ct-angular#51~ Resolved ~sand4rt/playwright-ct-angular#52~ Resolved

I have pulled and linked the PR branch and still face these errors. I also cannot import from @angular/material without adding "type": "module" to my package.json, or changing all my files to .mts.

Other than that it's working great so far. Thanks so much for this.

Hi @chronospatian, what is the value of ctViteConfig in the playwright config file? Did you try this:

    ctViteConfig: {
      plugins: [analog()], // or [swc.vite(swcAngularUnpluginOptions())]
      resolve: {
        /* @angular/material is using "style" as a Custom Conditional export to expose prebuilt styles etc... */
        conditions: ['style'],
      },
    },

yjaaidi avatar May 13 '24 08:05 yjaaidi

@rainerhahnekamp note that there are some known limitations on what can be used in providers. We've documented this here https://github.com/jscutlery/devkit/tree/main/packages/playwright-ct-angular#%EF%B8%8F-known-limitations but we will add this to playwright docs once this is merged.

yjaaidi avatar May 13 '24 08:05 yjaaidi