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

[5.7] Check for model in assertViewHas #26012

Merged
merged 1 commit into from Oct 9, 2018

Conversation

Projects
None yet
2 participants
@FatBoyXPC
Contributor

FatBoyXPC commented Oct 9, 2018

When using assertViewHas with a model it can end up comparing class state that the test may not care about, such as wasRecentlyCreated, pivots, or relationships, etc. The way I currently go about this is by passing a callback to assertviewHas:

$this->assertViewHas('user', function ($model) use ($user) {
    return $user->is($model);
});

I think being able to instead call it with assertViewHas('user', $user) would be a nice change, and I don't see any obvious negative consequences to this (unless the user cared about the 'hidden' state).

I'm not too cracked up about the test I added as it requires internal knowledge of $response, but I figure it's better than adding no tests. I could get rid of this line, but it would require rewriting assertViewHas to not be reliant on __get.

Edit: I have a couple custom assertions I would like to PR. They are related to each other, but not to this one - should I submit it as a separate PR or make it part of this one?

@TBlindaruk TBlindaruk changed the title from Check for model in assertViewHas to [5.7] Check for model in assertViewHas Oct 9, 2018

@taylorotwell taylorotwell merged commit 5b9f4c1 into laravel:5.7 Oct 9, 2018

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment