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
base: main
Are you sure you want to change the base?
Conversation
aa9b3fe
to
71599da
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
What's out plan here, is it ready to land? Is it based on Younes's work or is this something different? |
There was a problem hiding this 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.
Hey @pavelfeldman! I just shared my feedback with @sand4rt.
It doesn't matter anymore. There was just a misunderstanding. |
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 |
There was a problem hiding this 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.
720980b
to
71599da
Compare
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 |
71599da
to
8ccb3bb
Compare
This comment has been minimized.
This comment has been minimized.
I just noticed that the tests themselves are using Angular 15. |
Hey @sand4rt, I just created a This way, we can update the todo list as this:
Here are the advantages of moving the Angular vite plugin setup to the
The drawbacks are:
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. |
Hey @sand4rt are you available in the upcoming days or weeks so we can finish this 😊 |
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! |
Thanks @sand4rt! I think that we have everything now 😊:
Cf. sand4rt#5 |
9e88b09
to
31e8817
Compare
Great great great work! Thank you!! |
31e8817
to
3226aa5
Compare
This comment has been minimized.
This comment has been minimized.
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. |
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: sand4rt#5 (comment) I think it could either be supported like the other frameworks through the // 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');
}); |
Understood, then let's wait for the merge and then continue the discussion. Congrats again. |
@yjaaidi I am running into some issues while testing this library.
Other than that it's working great so far. Thanks so much for this. |
Hi @chronospatian, what is the value of 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'],
},
}, |
@rainerhahnekamp note that there are some known limitations on what can be used in providers. |
@yjaaidi I am using the same config as you have it here: https://github.com/sand4rt/playwright/blob/hello-angular-ct/tests/components/ct-angular/playwright.config.mts
|
|
@chronospatain you might want to check out #23662 for the esm/cjs issues |
The last thing I did to get hooks working was to make sure |
In which file did you put it initially? |
@yjaaidi I initially put it in the spec file, like beforeEach. |
Are there plans to add support for testing Directives and Pipes? eg. await mount(`<div [myDirective]="value | myPipe"></div>`, {
imports: [MyDirective, MyPipe],
props: {
value: "abc"
}
}) |
"typescript": "~5.2.0", | ||
"zone.js": "~0.14.0" | ||
}, | ||
"peerDependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be the first time we declare peerDependencies
, and we don't really like them. Where do we use them? Can we avoid declaring them as peers? Should we hard-depend on them, similar to depending on vite-plugin-solid
in @playwright/experimental-ct-solid
?
export type ComponentEvents = Record<string, Function>; | ||
|
||
export interface MountOptions<HooksConfig extends JsonObject, Component> { | ||
props?: Partial<Component> | Record<string, unknown>, // TODO: filter props and handle signals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you expand a bit on the TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The types are quite loose now while they should be stricter. @yjaaidi you already made something for this right?
|
||
/** @typedef {import('../playwright-ct-core/types/component').ObjectComponent} ObjectComponent */ | ||
|
||
import 'zone.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this one?
traverse(node, { | ||
enter: p => { | ||
// Treat calls to mount and all identifiers in arguments as component usages. | ||
// e.g. mount(MyComponent, { imports: [OtherComponent], providers: [Token]}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far, we like the idea of not depending on the usage, but only on declaration. It seems like Angular has a nice practice of putting components into Button.component.ts
files, so perhaps we can use this convention and treat .component
as a component file similar to .vue
and .svelte
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solely relying on the file extension might not be a good idea because of:
- Support for imports/providers in Angular is a must have (see this for more info)
- Native web components don't have a common file extension: (see this for more info)
- Vue components are not always defined in a
.vue
file (see this for more info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Could you elaborate on the imports/providers bit? I followed the link, and it seems to import AngularComponent
, presumably from the AngularComponent.component
file?
I also think that vue and lit are a different story, so I'd rather not mix them up in this PR aiming to support angular. We can have a separate discussion with pros and cons about it in #30269.
@@ -0,0 +1,45 @@ | |||
# See http://help.github.com/ignore-files/ for more about ignoring files. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an impressive gitignore file! Was it generated by angular cli for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, tests/components/ct-angular
was generated by angular cli
for (const hook of window.__pw_hooks_after_mount || []) | ||
await hook({ hooksConfig }); | ||
|
||
__pwFixtureRegistry.set(rootElement.id, fixture); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why mapping from id, and not from the rootElement
itself?
} from '@angular/platform-browser-dynamic/testing'; | ||
|
||
/** @type {WeakMap<import('@angular/core/testing').ComponentFixture, Record<string, import('rxjs').Subscription>>} */ | ||
const __pwOutputSubscriptionRegistry = new WeakMap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a WeakMap
, we usually use a symbol. However, in this case it seems like we can combine this map with __pwFixtureRegistry
?
// rootElement -> fixture, subscriptions
Map<Element, {
fixture: import('@angular/core/testing').ComponentFixture,
subscriptions: Record<string, import('rxjs').Subscription>
}>
throw new Error('Only standalone components are supported'); | ||
|
||
TestBed.configureTestingModule({ | ||
imports: [component.type], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we aim to support components as parameters (for slots?), do we have to list all component types in this imports
list?
The test fixture appears to be anchored to the wrong element. This causes some getters, matchers and locators to fail or behave unexpectedly. import { Component, ElementRef} from '@angular/core';
@Component({
standalone: true,
template: `<div>Test</div>`,
})
export class AttributeComponent {
constructor(element: ElementRef) {
element.nativeElement.setAttribute('name', 'hello')
}
} import { AttributeComponent } from '@/components/attribute.component';
import { expect, test } from '@playwright/experimental-ct-angular';
import type { HooksConfig } from 'playwright';
test('has name attribute', async ({ mount }) => {
const component = await mount<HooksConfig>(AttributeComponent);
// works
await expect(component.page().locator('#root').getAttribute('name')).resolves.toBe('hello');
// doesn't work
await expect(component.getAttribute('name')).resolves.toBe('hello');
// works
await expect(component.page().locator('#root')).toHaveAttribute('name', 'hello');
// doesn't work
await expect(component).toHaveAttribute('name', 'hello');
}); |
async function __pwRenderComponent(component) { | ||
const componentMetadata = reflectComponentType(component.type); | ||
if (!componentMetadata?.isStandalone) | ||
throw new Error('Only standalone components are supported'); | ||
|
||
TestBed.configureTestingModule({ | ||
imports: [component.type], | ||
}); | ||
|
||
await TestBed.compileComponents(); | ||
|
||
const fixture = TestBed.createComponent(component.type); | ||
fixture.nativeElement.id = 'root'; | ||
|
||
__pwUpdateProps(fixture, component.props); | ||
__pwUpdateEvents(fixture, component.on); | ||
|
||
fixture.autoDetectChanges(); | ||
|
||
return fixture; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestBed.createComponent
discards the root element, which causes the locator to break. Suggested refactor below should resolve this issue:
/**
* @param {ObjectComponent & MountOptions} component
* @param {HTMLElement} rootElement
*/
async function __pwRenderComponent(component, rootElement) {
const componentMetadata = reflectComponentType(component.type);
if (!componentMetadata?.isStandalone)
throw new Error('Only standalone components are supported');
TestBed.configureTestingModule({
imports: [component.type],
});
await TestBed.compileComponents();
const fixture = TestBed.createComponent(component.type)
rootElement.replaceChildren(fixture.nativeElement)
document.body.replaceChildren(rootElement)
__pwUpdateProps(fixture, component.props);
__pwUpdateEvents(fixture, component.on);
fixture.autoDetectChanges();
return fixture;
}
There was a problem hiding this 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.
Test results for "tests 1"1 flaky27343 passed, 662 skipped Merge workflow run. |
Test results for "tests 1"2 failed 2 flaky27009 passed, 610 skipped Merge workflow run. |
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.
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. |
closes: #14153 and https://github.com/sand4rt/playwright-ct-angular
TODO
Enable vite-plugin-angular JIT mode: feat(vite-plugin-angular): add support for JIT mode analogjs/analog#374 (comment)Sourcemap is likely to be incorrect: a plugin (@analogjs/vite-plugin-angular)
errors when transpiling: Sourcemap is likely to be incorrect: a plugin (@analogjs/vite-plugin-angular) analogjs/analog#410typeof plugin.default
check: https://github.com/microsoft/playwright/pull/27783/files#diff-4592ac823d44ec894c518ba459cfab4bd544056a674739fda5fc145cdc596923R28@analogjs/vite-plugin-angular
and related code and move it tocreate-playwright
? feat(ct): angular component testing #27783 (comment)