feat: add Angular Signals CT Harness for Angular 17.2 and up for users to be able to use Angular Signals within their component tests #29621

AtofStryker · 2024-06-05T15:16:10Z

Closes Setting input signals with Angular component tests not supported #29264

Additional details

Since signals introduced new methods/types to the API, we need to introduce a new test harness to Cypress in order to support signals. Right now we are calling it cypress/angular-signals. This will be readily available once this PR is merged in and cypress is released. In the future, we should be able to eventually merge this upstream into cypress/angular with the next major version of cypress when we remove/deprecate support for angular 13-16. In other words, the new harness need should not be permanent.

Documentation / Behavior

Typings issue

Mentioned in the issue is the following:

When attempting to set up a component test by mounting the Component directly, it fails to set the signal/model inputs through the componentProperties and gives incorrect type assertions on componentProperties.

This is because in our standard cypress/angular mount function, we expect a direct 1:1 mapping as the prop types

componentProperties?: Partial<{ [P in keyof T]: T[P] }>

In our new cypress/angular-signals harness, we need to make sure that a given generic, type T (or in the example given in the issue, a string) , can be inferred by an input(), signal(), or model() signal.

componentProperties?: Partial<{ [P in keyof T]: T[P] extends InputSignal<infer V> ? InputSignal<V> | WritableSignal<V> | V : T[P]}>

This way, specifying a string for an InputSignal<string> is a completely valid type.

Input Signal Behavior

Getting input() signals to work OOTB was a bit difficult, since we cannot create them outside of an angular context and there is no way to provide an injection context, even when setting an initial value. Because of this, Cypress handles input() signals when being injected into a component from the cypress context the following way:

If a prop is an input() signal, but the value provided is a match of the generic type, we wrap the generic value in a writable signal() and merge that into the prop. In other words:

  cy.mount(TestComponentComponent, {
    componentProperties: {
      title: 'Test Component',
    },
  });

is really

  cy.mount(TestComponentComponent, {
    componentProperties: {
      title: signal('Test Component'),
    },
  });

This allows us to make a signal and avoid the ctx.title is not a function error mentioned in #29264 while allowing the passing of primitives into the component to work.

Change spy for inputs

Since the prop in question is an input(), we do not propagate changes to titleChange or related output spies. In other words, this will not work:

  cy.mount(TestComponentComponent, {
    componentProperties: {
      title: 'Test Component',
      // @ts-expect-error
      titleChange: createOutputSpy('titleChange')
    },
  });

  // some action that changes the title...

  // this assertion will NEVER be true, even if you make `title` a signal. we will NEVER emit change events for props that are type `input()`
  cy.get('@titleChange').should('have.been.called');

Which brings up the question, how can I change or assert on input updates/changes?

If a prop is an input() signal, but the value provided is a match of the generic type wrapped in a signal, we merge the value as is to allow for one-way data binding (more on this below).

Since input() in our case can also take a WritableSignal, we can just pass a signal() as a variable reference into the component and mutate the signal directly. This gives us the one-way binding we need to test input() signals outside the context of Angular.

  const myTitleSignal = signal('Test Component')
  cy.mount(TestComponentComponent, {
    componentProperties: {
      title: myTitleSignal
    },
  });

cy.get('the-title-element').should('have.text', 'Test Component')
cy.then(() => {
   // now set the input() through a signal to update the one-way binding
   myTitleSignal.set('FooBar')
})
// works
cy.get('the-title-element').should('have.text', 'FooBar')

Model Signal Behavior

Since model() signals are writable signals, they have the ability to support two-way data binding. This means Cypress handles model() signals the following way:

If a prop is an modal() signal, but the value provided is a match of the generic type, we set the generic value in a model() and merge that into the prop. In other words:

  cy.mount(TestComponentComponent, {
    componentProperties: {
      count: 1
    },
  });

cy.get('the-count-element').should('have.text', '1')

Change spy for models

Since the prop in question is an model() and is a WritableSignal, we WILL propagate changes to countChange if the output spy is created, either like the example below or if autoSpyOutputs: true is configured. However, the typing for countChange is not supported since this is technically not a prop (@Output() actually adds this as a prop which is not true in our case, even though the signal output is completely valid).

  cy.mount(TestComponentComponent, {
    componentProperties: {
      count: 4,
      // @ts-expect-error
      countChange: createOutputSpy('countChange')
    },
  });

  // some action that changes the count...

  // this assertion will be true
  cy.get('@countChange').should('have.been.called');

However, since count is a primitive in this case and outside the angular context, we CANNOT support two-way data-binding to this variable. In other words, these will not work:

  let count = 5
  cy.mount(TestComponentComponent, {
    componentProperties: {
      count,
      // @ts-expect-error
      countChange: createOutputSpy('countChange')
    },
  });

  // some action that changes the count to 7 inside the component

  cy.then(() => {
    // this assertion will never be true. Count will be 5 
    expect(count).to.be(7)
  })
  // However, the change spy WILL be called since the change occurred inside the component

  let count = 5
  cy.mount(TestComponentComponent, {
    componentProperties: {
      count,
      // @ts-expect-error
      countChange: createOutputSpy('countChange')
    },
  });

  cy.then(() => {
   count = 8
  })
   // this assertion will never be true. Count will be 5 
  cy.get('the-count-element`).should('have.text`, '8')
   // the change spy will also NOT be called

Which brings up the question, how can I have two-way data-binding set up?

If a prop is an model() signal, but the value provided is a match of the generic type wrapped in a signal, we set the initial value and set up two-way data binding (more on this below). This means you can achieve two way data-binding in your component tests like such:

  let count = signal(5)
  cy.mount(TestComponentComponent, {
    componentProperties: {
      count
    },
  });

  // some action that changes the count to 7 inside the component

  cy.then(() => {
    // this assertion will be true
    expect(count()).to.be(7)
  })
  // Additionally, if registered, the change spy will also be called

  let count = signal(5)
  cy.mount(TestComponentComponent, {
    componentProperties: {
      count
    },
  });

  cy.then(() => {
   count.set(8)
  })
  // this assertion will be true
  cy.get('the-count-element').should('have.text', '8')
  // the change spy will also be called

  // later in the test , some action that changes the count to 14 inside the component
  // this assertion will be true
  cy.get('the-count-element').should('have.text', '14')

Steps to test

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation? feat: first pass at adding angular signals documentation cypress-documentation#5841 (approved)
Have API changes been updated in the type definitions?

npm/angular-signals/src/mount.ts

cypress · 2024-06-05T15:55:02Z

Passing run #55968 ↗︎

0	130	0	0	0

Details:

Merge branch 'develop' into angular-signals-ct-harness
Project: cypress	Commit: `8a8543211f`
Status: Passed	Duration: 17:35 💡
Started: Jun 28, 2024 2:55 PM	Ended: Jun 28, 2024 3:12 PM

Review all test suite changes for PR #29621 ↗︎

add changelog entry and build binary [run ci] rename angular18 to angular-signals until we are able to merge back into core package [run ci] fix linting job [run ci] make sure angular-signals harness is copied to cli after build [run ci] add project fixture directory to angular 18 and build binaries for newly named branch run ci update cache [run ci] bust nx cache [run ci] bust cache on linux [run ci] try busting the cache... again [run ci] usually helps when you have the correct build output... [run ci] fix issue where component internal props were getting blown away when user would not set prop in componentProperties [run ci]

…signals-ct-harness

jordanpowell88

We need to confirm a few of the items I left comments on before merging

npm/angular-signals/package.json

jordanpowell88 · 2024-06-12T19:54:27Z

system-tests/projects/angular-signals/src/signals-complex/signals-complex.component.html

+          <p data-cy="firstName"> {{ acquaintance.firstName }} </p>
+          <p data-cy="lastName"> {{ acquaintance.lastName }} </p>
+          <p data-cy="age"> {{ acquaintance.age }} </p>
+      </li>


Angular also introduced something called Control Flow that we should use moving forward. Angular's compiler handles this for us BUT it would be always ideal to validate its working correctly in our newest Angular CT libs.

I can add a system test to validate

added in 3f93d59

jordanpowell88 · 2024-06-12T19:56:12Z

system-tests/projects/angular-signals/src/signals-complex/signals-complex.component.ts

+    effect(() => {
+      // there is a bug in Angular 17 that doesn't rerender the signal when set outside the component context
+      // this is resolved in Angular 18. adding an effect() causes the template to be update when the signal is updated
+      console.log(`The user is: ${JSON.stringify(this.user())}`)


Are you wanting to keep these console.logs in here?

in this case yes, since it references the signal it forces a recalculation of the value and therefor a repaint. We don't need it in angular 18, but in 17 there is a bug where the signal being updated does not cause a repaint

jordanpowell88 · 2024-06-12T19:59:49Z

system-tests/projects/angular-signals/src/signals-optional/signals-optional.component.cy.ts

I'm assuming it is possible to spy on the output() without using createOutputSpy? We should add a test around this scenario at least once

cy.mount(SignalsOptionalComponent, { componentProperties: { title: 'Prop Title', count: 7, // @ts-expect-error countChange: cy.spy().as('countChange') },

for signal()s you need to create the output spy since we need to subscribe to the signal itself to propagate the event. The output isn't captured automatically unfortunately

jordanpowell88 · 2024-06-12T20:06:28Z

system-tests/projects/angular-signals/src/signals-required/signals-required.component.cy.ts

It would be great to test both scenarios (to and from) using the RxJs Interop with signals. Though it is in Developer Preview it is a key piece for most Angular devs.

We should be OK from this perspective since we are passing a raw signal down. The interop package can't be used outside of the angular injection context, so I don't think this is going to be a popular use case at least within the cypress spec file

jordanpowell88 · 2024-06-12T20:09:09Z

npm/angular-signals/src/mount.ts

@@ -0,0 +1,536 @@
+import 'zone.js'


We should do some validation around if including this is still necessary (I think it is). But we should also try to validate if mount works in zoneless

I tried to import this from core but couldn't find it in @angular/core. Looks like it might have been renamed to ɵprovideZonelessChangeDetection? Either way gave it a test here and seems to work ab8f852

jordanpowell88 · 2024-06-12T20:10:59Z

npm/angular-signals/src/mount.ts

+   * @memberof MountConfig
+   * @description flag defaulted to true to automatically detect changes in your components
+   */
+  autoDetectChanges?: boolean


We probably don't need this anymore with signals

we still need it for the fixture update from what I can tell

jordanpowell88 · 2024-06-12T20:23:52Z

npm/angular-signals/src/mount.ts

+      // update the model signal with the properties updates
+      toObservable(propValue, {
+        injector,
+      }).subscribe((value) => {


We should validate their isn't a memory leak here and that we don't need to unsubscribe (or that it is unsubscribed automatically when we teardown before each test)

This was a memory leak. The observable by the looks of it hangs around because the subscription is active. To fix this, I pushed the subscription into a list to clean up when we clean up the fixture. Updated and fixed in 12c7e36. Really good catch!

…signals-ct-harness

system-tests/projects/angular-signals/.gitignore

ryanthemanuel · 2024-06-24T14:10:46Z

system-tests/projects/angular-signals/cypress/support/component-index.html

+  <body>
+    <div data-cy-root></div>
+  </body>
+</html>


Suggested change

</html>

</html>

ryanthemanuel · 2024-06-24T14:11:57Z

system-tests/projects/angular-signals/src/signals-required/signals-required.component.cy.ts

+  cy.get('[data-cy="signals-required-component-title-display"]').should('contain.text', 'Signals Component as Primitive')
+})
+
+// FIXME: we currently need to allow this as there isn't a great way to set input signals in a component due to how input signals are instantiated


Do we have an issue logged for this work?

logged #29732

added a comment too in 001813e

npm/angular-signals/src/mount.ts

ryanthemanuel · 2024-06-24T14:14:38Z

npm/angular-signals/src/mount.ts

+  }
+}
+
+// currently not a great way to detect if a function is an InputSignal. When we discover a better way to detect a signal we should incorporate it here.


Do we want to log an issue to track on finding better ways to determine this stuff?

We probably should. I will create one!

logged #29731

also updated the comments in 001813e

…signals-ct-harness

jordanpowell88

PR looks good to me. Thanks for putting this together @AtofStryker

ryanthemanuel

Nice work @AtofStryker!

cypress-bot · 2024-07-01T20:52:22Z

Released in 13.13.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.13.0, please open a new issue.

AtofStryker commented Jun 5, 2024

View reviewed changes

npm/angular-signals/src/mount.ts Outdated Show resolved Hide resolved

AtofStryker force-pushed the angular-signals-ct-harness branch 3 times, most recently from ace8fa2 to 30d95ad Compare June 5, 2024 18:49

AtofStryker mentioned this pull request Jun 5, 2024

Setting input signals with Angular component tests not supported #29264

Closed

AtofStryker force-pushed the angular-signals-ct-harness branch from 1a23468 to 00c2884 Compare June 6, 2024 13:10

Base automatically changed from feat/support_angular_18 to develop June 7, 2024 16:19

AtofStryker added 2 commits June 7, 2024 12:20

direct copy paste of angular npm package to angular18 (no changes)

36eac85

AtofStryker force-pushed the angular-signals-ct-harness branch from d0ebc25 to c6cba8b Compare June 7, 2024 16:24

AtofStryker self-assigned this Jun 7, 2024

AtofStryker marked this pull request as ready for review June 7, 2024 18:50

AtofStryker requested review from jordanpowell88, ryanthemanuel and jennifer-shehane June 7, 2024 18:51

AtofStryker mentioned this pull request Jun 13, 2024

feat: first pass at adding angular signals documentation cypress-io/cypress-documentation#5841

Merged

Merge branch 'develop' of github.com:cypress-io/cypress into angular-…

e0627f7

…signals-ct-harness

AtofStryker requested a review from mschile June 19, 2024 15:38

jordanpowell88 suggested changes Jun 19, 2024

View reviewed changes

AtofStryker added 3 commits June 19, 2024 11:59

update test harness description

b6d6f7e

add built-in control flow tests with signals

3f93d59

make sure to clean up rxjs observables in order to prevent a memory leak

12c7e36

AtofStryker requested a review from jordanpowell88 June 20, 2024 14:40

Merge branch 'develop' of github.com:cypress-io/cypress into angular-…

4b1e648

…signals-ct-harness

ryanthemanuel reviewed Jun 24, 2024

View reviewed changes

system-tests/projects/angular-signals/.gitignore Outdated Show resolved Hide resolved

ryanthemanuel reviewed Jun 24, 2024

View reviewed changes

npm/angular-signals/src/mount.ts Show resolved Hide resolved

ryanthemanuel reviewed Jun 24, 2024

View reviewed changes

AtofStryker added 2 commits June 24, 2024 14:01

address comments from code review

001813e

Merge branch 'develop' of github.com:cypress-io/cypress into angular-…

6090456

…signals-ct-harness

AtofStryker requested a review from ryanthemanuel June 24, 2024 18:04

jordanpowell88 approved these changes Jun 24, 2024

View reviewed changes

ryanthemanuel approved these changes Jun 25, 2024

View reviewed changes

Merge branch 'develop' into angular-signals-ct-harness

9bbed67

jennifer-shehane removed request for jennifer-shehane and mschile June 25, 2024 14:48

jennifer-shehane and others added 2 commits June 25, 2024 10:50

yarn lock

fff8222

Merge branch 'develop' into angular-signals-ct-harness

8a85432

AtofStryker merged commit f2554f1 into develop Jun 28, 2024
121 of 129 checks passed

AtofStryker deleted the angular-signals-ct-harness branch June 28, 2024 17:37

cypress-bot bot locked as resolved and limited conversation to collaborators Jul 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add Angular Signals CT Harness for Angular 17.2 and up for users to be able to use Angular Signals within their component tests #29621

feat: add Angular Signals CT Harness for Angular 17.2 and up for users to be able to use Angular Signals within their component tests #29621

AtofStryker commented Jun 5, 2024 •

edited

Loading

cypress bot commented Jun 5, 2024 •

edited

Loading

jordanpowell88 left a comment

jordanpowell88 Jun 12, 2024

AtofStryker Jun 19, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

jordanpowell88 Jun 12, 2024

AtofStryker Jun 20, 2024

ryanthemanuel Jun 24, 2024

ryanthemanuel Jun 24, 2024

AtofStryker Jun 24, 2024

AtofStryker Jun 24, 2024

ryanthemanuel Jun 24, 2024

AtofStryker Jun 24, 2024

AtofStryker Jun 24, 2024

AtofStryker Jun 24, 2024

jordanpowell88 left a comment

ryanthemanuel left a comment

cypress-bot bot commented Jul 1, 2024

feat: add Angular Signals CT Harness for Angular 17.2 and up for users to be able to use Angular Signals within their component tests #29621

feat: add Angular Signals CT Harness for Angular 17.2 and up for users to be able to use Angular Signals within their component tests #29621

Conversation

AtofStryker commented Jun 5, 2024 • edited Loading

Additional details

Documentation / Behavior

Typings issue

Input Signal Behavior

Change spy for inputs

Model Signal Behavior

Change spy for models

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented Jun 5, 2024 • edited Loading

Passing run #55968 ↗︎

Review all test suite changes for PR #29621 ↗︎

jordanpowell88 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jordanpowell88 left a comment

Choose a reason for hiding this comment

ryanthemanuel left a comment

Choose a reason for hiding this comment

cypress-bot bot commented Jul 1, 2024

AtofStryker commented Jun 5, 2024 •

edited

Loading

cypress bot commented Jun 5, 2024 •

edited

Loading