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

[6.x] Add assertJsonPathStrict method #30152

Closed
wants to merge 1 commit into from
Closed

[6.x] Add assertJsonPathStrict method #30152

wants to merge 1 commit into from

Conversation

Arkanius
Copy link
Contributor

@Arkanius Arkanius commented Oct 1, 2019

Implements the suggestion of @BrandonSurowiec and @ttrig at #30142

This function is an alias of assertJsonPath. The tests are just the same

@SjorsO
Copy link
Contributor

SjorsO commented Oct 1, 2019

It is probably a better idea to make assertJsonPath strict by default. @ttrig gave a good example of the results you can get when you use this assertion in a non-strict way:

{
    "foo": false
}

$response->assertJsonPath('foo', false); // pass
$response->assertJsonPath('foo', 0); // pass
$response->assertJsonPath('foo', null); // pass

You should never want behavior like this in a test.

@Arkanius
Copy link
Contributor Author

Arkanius commented Oct 1, 2019

You have a point. I think the best place to talk about it is at the original PR, #30142

@Arkanius Arkanius changed the title [6.x] Strict parameter added to assertJsonPath [6.x] Add assertJsonPathStrict method Oct 1, 2019
@ahinkle
Copy link
Contributor

ahinkle commented Oct 1, 2019

Doesn't really add any benefit. assertJson, which has an optional strict argument, doesn't have its own strict method.

@Arkanius
Copy link
Contributor Author

Arkanius commented Oct 1, 2019

Well, this comportament is wierd to me. I implemented the suggestion by thinking that laravel has some methods like this.

If this is really not relevant I'll close the pr.

@ttrig
Copy link
Contributor

ttrig commented Oct 2, 2019

Doesn't really add any benefit. assertJson, which has an optional strict argument, doesn't have its own strict method.

One benefit could be the readability of assertJsonPath('foo', false, true) compared to assertJsonPathStrict('foo', false).

Would be nice until assertJsonPath is strict by default.

@taylorotwell
Copy link
Member

No need.

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.

5 participants