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

Core: Ensure that context.args is always set #16790

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

tmeasday
Copy link
Member

Issue: #16743

What I did

Ensure we don't filter all args down to nothing.

How to test

  • Is this testable with Jest or Chromatic screenshots?
    Yes see tests

@tmeasday tmeasday requested a review from shilman November 26, 2021 02:54
@nx-cloud
Copy link

nx-cloud bot commented Nov 26, 2021

Nx Cloud Report

CI ran the following commands for commit 35d0096. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@shilman shilman changed the title Ensure that context.args is always set. Core: Ensure that context.args is always set Nov 26, 2021
@shilman shilman added this to the 6.4 PRs milestone Nov 26, 2021
hooks: new HooksContext(),
...firstStory,
} as any);
expect(renderMock).toHaveBeenCalledWith({}, expect.objectContaining({ argsByTarget: {} }));
Copy link
Member

Choose a reason for hiding this comment

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

@tmeasday shouldn't this be asserting something about args? and same for above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Args is the first argument ({})

Copy link
Member

Choose a reason for hiding this comment

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

aha! 💡

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Successfully merging this pull request may close these issues.

2 participants