FundingCircle / jackdaw

A Clojure library for the Apache Kafka distributed streaming platform.
https://fundingcircle.github.io/jackdaw/
BSD 3-Clause "New" or "Revised" License
369 stars 80 forks source link

Formalizing the test-machine command API #230

Closed cddr closed 4 years ago

cddr commented 4 years ago

Rationale

The test-machine commands are "just data". This allows them to be created using whatever method is convenient (e.g. generated from the contents of some edn file, extracted from a database, read from an existing kafka topic etc.). However when authoring commands from scratch, it can be annoying to have to remember the syntax of each command (or keep referring back to the docs).

This PR adds some functions in the jackdaw.test.commands ns to provide a bit of help for this use-case. Having (spec'd) functions means that tools like cider can give us a bit of interactive help without the need to leave our editor. The specs are very basic for now but we can further refine them (particularly the ::write-options and ::watch-options specs) to provide even better assistance. As an example, the screenshot below is the result from M-x cider-doc on the jackdaw.test.commands/watch function.

Screen Shot 2020-02-12 at 5 01 17 PM

Checklist

codecov[bot] commented 4 years ago

Codecov Report

Merging #230 into master will increase coverage by 0.09%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
+ Coverage   80.20%   80.30%   +0.09%     
==========================================
  Files          42       42              
  Lines        2627     2660      +33     
  Branches      151      153       +2     
==========================================
+ Hits         2107     2136      +29     
- Misses        369      371       +2     
- Partials      151      153       +2     

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d11762b...8cbc24d. Read the comment docs.

99-not-out commented 4 years ago

Nice - good start to add hooks for further stuff, and the spec is a very helpful addition.

AndreaCrotti commented 4 years ago

Nice work! One question, if I read correctly all the spec validation will only be done if you use the new wrappers? I think that maybe that adding a s/assert where the command gets executed (jd.test/run-test for example) could also be good, since it would check that instructions are well formed also when they are not generated using the wrappers.

cddr commented 4 years ago

I think that maybe that adding a s/assert where the command gets executed (jd.test/run-test for example) could also be good, since it would check that instructions are well formed also when they are not generated using the wrappers.

Yeah I think it's worth following up with that. The :ret specs here are pretty minimal (i.e. they just specify vector?) so if we make them a bit more stringent by specifying the structure of the returned vector, then we can re-use those specs in run-test.