Feature/dev harrel impl

[harrel56]

Reviewer checklist

• Read the contributing guide
• PR should be motivated, i.e. what does it fix, why, and if relevant, how
• Ensure relevant issues are linked (description should include text like "Fixes #")
• Ensure any appropriate documentation has been added or amended

[harrel56]

Hi, let me know if there is something missing. I have no suggestion though: could you somehow separate spec compliance to required & optional? Because now it feels a little bit unfair as I have the highest score for required and the lowest score for overall. I just don't support format assertions (yet?) which is most of the optional tests are checking.

Also I think the serde tests might be a bit unfair? E.g. "Medeia" doesn't support newest drafts so it is run with draft7 and it cannot really be compared to implementations which are run with draft2020-12. Probably this also should be grouped by specs?

[harrel56]

And just out of curiosity: how long does it take you to run all benchmarks? :P It looks like it might take a whole night or so

[big-andy-coates]

Thanks @harrel56, I'll take a look at this.

could you somehow separate spec compliance to required & optional?

Results are split between required and optional. Though currently the graph is not.

The table summarising the functional results breaks them down into pass/fail counts on optional and rrequired features.

The over all score treats blends both required and optional, with required test cases making up 75% of the score.

It would be possible to add a graph comparing only required functional coverage. Feel free to raise an issue, and if you think its important enough, ideally work on it :).

Also I think the serde tests might be a bit unfair? E.g. "Medeia" doesn't support newest drafts so it is run with draft7 and it cannot really be compared to implementations which are run with draft2020-12.

Yes, I'm aware of this. Though it would be great if you could raise this as an issue.

I'm currently working on making the charts and tables of results be driven from the latest test runs, so that results update as new versions of validator libraries come out. As part of this work, I have a todo to call out which draft version the serde benchmark is using, what this means, and to potentially break it down by spec.

Time is always a limiting factor though ;)

[harrel56]

When the benchmarks and graphs are going to be automatically generated I'll be happy to help out with splitting this graph - I just don't wanna do it manually. Gonna report this as a separate issue then

[big-andy-coates]

Hey @harrel56,

First off, a big apology for committing a refactor while you were in the middle of your work. My bad. I'm just trying to finish off a few things to make results more data driven.

This looks great!

I've made a few comments below. Mainly looks like stuff due to you refactoring the code after I changed things. A bit of clean up and should be good to go.

[big-andy-coates]

And just out of curiosity: how long does it take you to run all benchmarks? :P It looks like it might take a whole night or so

Yeah, the benchmarks take a while to run! They run in a GitHub workflow.

There is a smoke test for the functional tests that runs as part of the build though, to catch any issues.

[big-andy-coates]

### Conflicting files
src/main/java/org/creekservice/kafka/test/perf/JsonSerdeBenchmark.java
src/main/java/org/creekservice/kafka/test/perf/JsonValidateBenchmark.java

Sorry - forgot I moved those files! They are now under performance package.

[harrel56]

First off, a big apology for committing a refactor while you were in the middle of your work. My bad. I'm just trying to finish off a few things to make results more data driven.

Yeah, that really wasn't a problem so no worries :)

I think I addressed all your comments - let me know if there's anything left

[big-andy-coates]

LGTM @harrel56

Few minors above, plus the following remaining from the previous reviews:

• Feature/dev harrel impl #50 (comment)
• Feature/dev harrel impl #50 (comment)

[harrel56]

Ok, completed another iteration - it's getting a little messy, hopefully didn't omit anything

[big-andy-coates]

LGTM!

Thanks for your contribution.

In case you missed them, there are two follow on comments you may be interested in:

• Feature/dev harrel impl #50 (comment)
• Feature/dev harrel impl #50 (comment)

Functional results are up: https://www.creekservice.org/json-schema-validation-comparison/functional

Performance results are working locally - pushing soon.

[harrel56]

Nice, thanks!

Functional results are up: https://www.creekservice.org/json-schema-validation-comparison/functional

Hate to be dead last here - looking forward to required & optional separation! BTW I messaged you on slack regarding this

[big-andy-coates]

Performance results are up: https://www.creekservice.org/json-schema-validation-comparison/performance