Skip to content

[POC] Add Expectation::isJson() to verify json string and decode - #19

Draft
kbond wants to merge 1 commit into
zenstruck:1.xfrom
kbond:is-json
Draft

[POC] Add Expectation::isJson() to verify json string and decode#19
kbond wants to merge 1 commit into
zenstruck:1.xfrom
kbond:is-json

Conversation

@kbond

Copy link
Copy Markdown
Member

Per a slack discussion with @nikophil.

Assert::that('[4, 5, 6]')
->isJson() // json_decode's the current value and starts a new expectation with this
->contains(5)
;
Assert::that('5')
->isJson() ->is(5)
->isGreaterThan(4)
;

@kbondkbond added the enhancement New feature or request label Jul 11, 2022
@kbond

Copy link
Copy Markdown
MemberAuthor

Not sure we want to do this but wanted to capture our conversation with a POC. We could go even further and move the Json class from zenstruck/browser here (and have Expectation::isJson(): Json).

Comment threadsrc/Assert/Expectation.php
*
* Fails if not a valid json string.
*/
public function isJson(): self

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.

not sure of the name: it sounds like it's just an expectation, but it does more... decodeJson() ?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yeah, this tripped me up as well. Technically it is also an expectation but is not clear that it changes the underlying data.

I chose this purely because the api reads better: "assert that value is json" vs "assert that value decode json". I definitely probably get too hung up on these semantics 😄.

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

Labels

enhancementNew feature or request

Development

Successfully merging this pull request may close these issues.

2 participants

@kbond@nikophil