Skip to content

docs: update attribute testing guide to include canonical example wit… - #69878

Merged
alxhub merged 1 commit into
angular:mainfrom
mfstapert-xip:docs/update-directives-testing-guide
Jul 24, 2026
Merged

alxhub merged 1 commit into
angular:mainfrom
mfstapert-xip:docs/update-directives-testing-guide

Conversation

@mfstapert-xip

Copy link
Copy Markdown
Contributor

PR Checklist

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Current guide on testing directives does not provide a explicit example of unit testing directives with new input signals syntax.

What is the new behavior?

This doc contains an example on how to unit test a directive using a local component to drive the directive.

Does this PR introduce a breaking change?

No

Other information

Reason for adding this is this thread: #54164
Hopefully this closes this issue.

@google-cla

google-cla Bot commented Jul 21, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@pullapprove
pullapprove Bot requested a review from kirjs July 21, 2026 21:37
@angular-robot angular-robot Bot added the area: docs Related to the documentation label Jul 21, 2026
@ngbot ngbot Bot added this to the Backlog milestone Jul 21, 2026
Comment on lines +187 to +194
it("should update the directive's input", async () => {
fixture.componentRef.setInput('color', 'blue');
await fixture.whenStable();

const debugElement = fixture.debugElement.query(By.directive(Highlight));
const highLight = debugElement.injector.get(Highlight);
expect(highLight.bgColor()).toBe('blue');
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i would drop that test which is a bad example. It's basically testing the implementation rather than the feature it provides.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, this also makes the last sentence redundant


it('should fall back to the default color when no color is provided', () => {
const p = fixture.nativeElement.querySelector('p');
expect(p.style.backgroundColor).toBe('rgb(211, 211, 211)'); // default light gray color

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That test would fail, as we don't have a default color. We could set the input default value to it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ran this locally and it should pass with the directive at the top of this page. But this might be confusing to readers who only look for this section so I dropped this case.

@mfstapert-xip
mfstapert-xip force-pushed the docs/update-directives-testing-guide branch from cf9c2e2 to 407fc61 Compare July 22, 2026 14:01
@mfstapert-xip
mfstapert-xip requested a review from JeanMeche July 22, 2026 16:15
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jul 23, 2026
@JeanMeche
JeanMeche removed the request for review from kirjs July 23, 2026 21:11
@alxhub
alxhub merged commit 738b8fe into angular:main Jul 24, 2026
25 checks passed
@alxhub

alxhub commented Jul 24, 2026

Copy link
Copy Markdown
Member

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: docs Related to the documentation target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants