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

TODO: add testcase from BOLT07 for extended-queries.json #64

Open
joemphilips opened this issue Mar 1, 2020 · 7 comments
Open

TODO: add testcase from BOLT07 for extended-queries.json #64

joemphilips opened this issue Mar 1, 2020 · 7 comments
Labels
CI/Tests This issue is related to CI/Test pipeline. enhancement New feature or request good first issue Good for newcomers

Comments

@joemphilips
Copy link
Owner

I've just realized that BOLT has json file of testcase for extended queries here

Right now, it only has property tests. It will be good starting point if someone wants to start understanding this library and F# / Expecto / System.Text.Json in general.
Imitating TLVSerialize.fs might be good way to get done.

@joemphilips joemphilips added CI/Tests This issue is related to CI/Test pipeline. enhancement New feature or request good first issue Good for newcomers labels Mar 1, 2020
@joemphilips joemphilips changed the title TODO: add testcase from in BOLT07 for extended-queries.json TODO: add testcase from BOLT07 for extended-queries.json Mar 1, 2020
@knocte
Copy link
Collaborator

knocte commented Mar 1, 2020

Why System.Text.Json instead of Newtonsoft.Json?

@joemphilips
Copy link
Owner Author

Just for the sake of consistency and performance. It is fine to use Newtonsoft as well.

@knocte
Copy link
Collaborator

knocte commented Mar 1, 2020

Consistency with what?
WRT the latter: IMO correctness is better.

@joemphilips
Copy link
Owner Author

Consistancy with other part of the test code.
Correctness does not change by witch json library we use.

@knocte
Copy link
Collaborator

knocte commented Mar 2, 2020

Consistency with other part of the test code.

This made me realise I was misunderstanding our conversation. It turns out the first time I read this issue I misread "System.Text.Json" as "ServiceStack.Json" (https://www.nuget.org/packages/ServiceStack.Text/). Now it's all clear.

Correctness does not change by witch json library we use.

You would be surprised! I was actually raising this because I know cases of differing behaviour between ServiceStack.Json and Newtonsoft.Json (that would make you understand why the former is actually faster).

@knocte
Copy link
Collaborator

knocte commented Mar 2, 2020

I know cases of differing behaviour between ServiceStack.Json and Newtonsoft.Json

One example (and this is just one of them): https://stackoverflow.com/questions/11882987/why-servicestack-text-doesnt-default-dates-to-iso8601

@joemphilips
Copy link
Owner Author

Interesting. Well, in this case it is just for reading json from flat file so I don't think it is a big deal. But I got your point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/Tests This issue is related to CI/Test pipeline. enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants