kinbiko / jsonassert

A Go test assertion library for verifying that two representations of JSON are semantically equal
MIT License
125 stars 16 forks source link

[BUG] Assert fails when expectedjson contains '%' #27

Closed kallyshall closed 3 years ago

kallyshall commented 3 years ago

What exactly did you do?

ja.Assertf(`{"test":"A%AAA%BBBB%CC"}`, `{"test":"A%AAA%BBBB%CC"}`)

I think this happens because the fmt.Sprintf used in the package

What did you expect would happen?

I expect assert pass in this situation my guess is on the fmt.Sprintf used in Assertf function, It identifies the %char as some unsupported formatter

What actually happened?

Assertf format %A has unknown verb A

Additional info

kinbiko commented 3 years ago

Thanks for highlighting this issue, and apologies for getting back to you so late.

I think this behaviour is common to all ...f functions, like t.Errorf and fmt.Fprintf. ja.Assertf is also such a ...f function, but unlike these other examples this package doesn't (yet) provide a non-f version. It probably should.

I think adding this feature itself would be trivial (new exported function Assert without the fmt.Sprintf call, everything else the same). I'll have a quick look and try and create a new release this weekend, otherwise I would welcome a PR if anyone feels like adding new tests, examples, and docs.

kinbiko commented 3 years ago

On further thought, I think there might already be a reasonable workaround in place:

ja.Assertf(`{"test":"A%AAA%BBBB%CC"}`, `{"test":"%s"}`, "A%AAA%BBBB%CC")

I'll probably just update the docs describing how to handle this edge-case.

Thanks again for bringing this up!

kallyshall commented 3 years ago

On further thought, I think there might already be a reasonable workaround in place:

ja.Assertf(`{"test":"A%AAA%BBBB%CC"}`, `{"test":"%s"}`, "A%AAA%BBBB%CC")

I'll probably just update the docs describing how to handle this edge-case.

Thanks again for bringing this up!

thanks for looking into it XD and providing a good workaround c'',)