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.

Sign up for GitHub

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

Jump to bottom

Make checking's options optional #58

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Make checking's options optional #58

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e77ca7e
checking can take no option and will use quick-check's default (first…
DjebbZ Apr 3, 2018
dd5ad5f
Small refinements to code and tests
DjebbZ Apr 4, 2018
2dd538a
Update documentation
DjebbZ Apr 4, 2018
810782c
Fix error throwing for cljs
DjebbZ Apr 4, 2018
03240d7
Indentation and parens fixes after code review. Node tests still on e…
DjebbZ Apr 5, 2018
8a8085d
Fix node test, other fixes after 2nd review, new test KO in clj (OK c…
DjebbZ Apr 5, 2018
81ce719
Explicit reference to defspec options in `testing` docstring
DjebbZ Apr 6, 2018
0939c8a
Test that catch exception with bad options works OK in CLJ and CLJS \o/
DjebbZ Apr 6, 2018
122068e
Changes after review; still got some macro problems
DjebbZ Apr 16, 2018
ff48572
Quoted binding to avoid passing fully-qualified name
DjebbZ Apr 16, 2018
a11771a
Fix integer? and try fixing remaining uncaught exception
DjebbZ May 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions src/com/gfredericks/test/chuck/clojure_test.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
(ct/report reports))

(defmacro checking
^{:doc "A macro intended to replace the testing macro in clojure.test with a
{:doc "A macro intended to replace the testing macro in clojure.test with a
generative form. To make (testing \"doubling\" (is (= (* 2 2) (+ 2 2))))
generative, you simply have to change it to
(checking \"doubling\" [x gen/int] (is (= (* 2 x) (+ x x)))).
Expand All @@ -123,19 +123,20 @@

For background, see
http://blog.colinwilliams.name/blog/2015/01/26/alternative-clojure-dot-test-integration-with-test-dot-check/"
:arglists '([name bindings body] [name num-tests-or-options bindings body])}
:arglists '([name bindings & body] [name num-tests-or-options bindings & body])}
[name & check-decl]
(let [[num-tests-or-options bindings body] (cond
(and (or number? (first check-decl)
map? (first check-decl))
(vector? (second check-decl)))
[(first check-decl) (second check-decl) (nnext check-decl)]

(vector? (first check-decl))
[nil (first check-decl) (next check-decl)]

:else (throw #?(:clj (IllegalArgumentException. "Arguments to `checking` must be either [name bindings body] or [name num-tests-or-options bindings body]")
:cljs (js/Error. "Arguments to `checking` must be either [name bindings body] or [name num-tests-or-options bindings body]"))))
(let [[num-tests-or-options bindings body]
(cond
(and (or (number? (first check-decl))
(map? (first check-decl)))
(vector? (second check-decl)))
[(first check-decl) (second check-decl) (nnext check-decl)]

(vector? (first check-decl))
[nil (first check-decl) (next check-decl)]

:else (throw #?(:clj (IllegalArgumentException. "Arguments to `checking` must be either [name bindings body] or [name num-tests-or-options bindings body]")
:cljs (js/Error. "Arguments to `checking` must be either [name bindings body] or [name num-tests-or-options bindings body]"))))
Copy link
Owner

Choose a reason for hiding this comment

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

you can avoid the duplication here by pushing the #? down to just the IllegalArgumentException. and js/Error.

num-tests-or-options (tc.clojure-test/process-options num-tests-or-options)]
Copy link
Owner

Choose a reason for hiding this comment

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

the fact that you're calling process-options with the form at macroexpansion-time means that users can't have a non-literal expression; e.g., I think (checking "something" (assoc some-defaults :num-tests 40) ...) would fail (you could add a test for this).

Now that I think about that distinction, I believe your check for number? and map? farther up are not sufficient, since it could also be a symbol or sequence. I think the cleanest thing would just be to only check (vector? (second check-decl)) there.

Copy link
Author

Choose a reason for hiding this comment

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

Done, but... (read the comment below)

`(-testing ~name
(fn []
Expand Down
6 changes: 3 additions & 3 deletions test/com/gfredericks/test/chuck/clojure_test_test.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
;; no option is OK, defaults to 100 tests
(let [nb-runs (atom 0)]
(checking "no option works" [i gen/s-pos-int]
(swap! nb-runs inc)
(is (> 0)))
(swap! nb-runs inc)
(is (> 0)))
Copy link
Owner

Choose a reason for hiding this comment

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

this isn't really core to the test, but I assume you meant (> i 0) or (pos? i) here?

(testing "no option means 100 runs (test.check's default)"
(is (= 100 @nb-runs))))
;; empty map is OK, defaults to 100 tests
(checking "strings are strings" {} [s gen/string-ascii]
(is (string? s)))
(is (string? s)))
;; passes because the number of tests is small
(checking "small ints" {:num-tests 5} [i gen/s-pos-int]
(is (< i 10)))
Expand Down