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

[9.x] Add view data assertions to TestView #43923

Merged
merged 2 commits into from
Aug 30, 2022
Merged

Conversation

browner12
Copy link
Contributor

  • assertViewHas()
  • assertViewHasAll()
  • assertViewMissing()

these methods are all available on the TestResponse, but not currently TestView. These assertions can be more helpful to test the workings of the component, and not reliant on how the data actually gets displayed on the view side of things.

- `assertViewHas()`
- `assertViewHasAll()`
- `assertViewMissing()`

these methods are all available on the `TestResponse`, but not currently `TestView`.  These assertions can be more helpful to test the workings of the component, and not reliant on how the data actually gets displayed on the view side of things.
@taylorotwell
Copy link
Member

If this is a test view object, is it a bit redundant to say assertViewHas instead of just assertHas?

@browner12
Copy link
Contributor Author

possibly, although it might be nice to explicitly know we're targeting the View and not the rendered content.

these methods where copied and tweaked from the TestResponse object, which is why they got the names they did.

either way, your call. happy to make the changes if desired.

@taylorotwell taylorotwell merged commit e07e005 into laravel:9.x Aug 30, 2022
@browner12 browner12 deleted the 9.x branch August 30, 2022 18:34
@browner12 browner12 restored the 9.x branch August 30, 2022 18:34
@browner12 browner12 deleted the 9.x branch August 30, 2022 18:34
@browner12 browner12 restored the 9.x branch August 30, 2022 18:34
@jasonmccreary
Copy link
Contributor

Tiny, tiny papercut, but if this is new code, I'd love to be able to check a piece of view data is explicitly null (as you can't in the current assertions since null is the default value and ignored).

@browner12
Copy link
Contributor Author

this just got deployed today, so unfortunately can't change it now.

I'm trying to think how this would work, and AFAIK in PHP you can't tell if an argument is passed, or if it's using the default value. I feel like we'd have to separate out a separate method for checking simply if the variable is passed, and one for if the variable has a specific value.

@jasonmccreary
Copy link
Contributor

jasonmccreary commented Sep 6, 2022

Yeah, no worries. Just something in the rare times I've actually passed in null as view data I thought could be improved.

I've seen other languages use a constant in this case. For example, Objective-C uses a NOT_FOUND constant when searching strings. Mimicking that, something like a TestView::IGNORE_VALUE could be the default - allowing null to be passed in explicitly.

@browner12
Copy link
Contributor Author

very interesting.... I feel like this could be super beneficial in numerous places on the framework.

how do you check for the constant? aren't you still comparing by value? what value do you assign the constant?

@jasonmccreary
Copy link
Contributor

Yes, its actual value could be a bit tricky as a simple value could pose the same problem. I suppose that's the benefit of a language constant.

Nonetheless, something random would have a lower hit rate than null. For example, LARAVEL_CONSTANT_256. 😂

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

Successfully merging this pull request may close these issues.

3 participants