RackSec / desdemona

Data-backed security operations
Eclipse Public License 1.0
2 stars 7 forks source link

Clean up test, factoring is's -> are #41

Closed ehashman closed 8 years ago

ehashman commented 8 years ago

Was reading about testing in Clojure (is and are) and skimming through our tests. I thought this might look a bit cleaner?

lvh commented 8 years ago

Usually are is for when you have several test cases for the same pure fn, so this is a somewhat uncommon use case for it, but it's not bad :) I think the error message when it fails might be a little less instructive (and tooling will show the are as the failing test, instead of the specific line that failed).

ehashman commented 8 years ago

Yeah I noticed that, and wasn't sure which way was actually better, so I thought I'd just submit this and see what people thought.

codecov-io commented 8 years ago

Current coverage is 52.98%

Merging #41 into master will not affect coverage as of 63bf810

@@            master     #41   diff @@
======================================
  Files           17      17       
  Stmts          285     285       
  Branches         6       6       
  Methods          0       0       
======================================
  Hit            151     151       
  Partial          6       6       
  Missed         128     128       

Review entire Coverage Diff as of 63bf810

Powered by Codecov. Updated on successful CI builds.

derwolfe commented 8 years ago

I'd vote for using is instead of are. The only reason being that is will produce a more descriptive error in the event of a test failure. are is helpful when there are a large amount of tests performing the same type of assertion; instead of materially different tests. :-)

A few examples:

using are (the current patch), the test failure triggers on line 23, where (= x y) is declared

Chris@juniper ~/C/desdemona (test-cleanup)> lein test

lein test desdemona.functions.sample-functions-test

lein test desdemona.jobs.sample-job-test

lein test :only desdemona.jobs.sample-job-test/build-job-test

FAIL in (build-job-test) (sample_job_test.clj:23)
expected: (= lifecycles expected-lifecycles)
  actual: (not (= ({:lifecycle/task :write-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls} {:lifecycle/task :read-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls} {:lifecycle/task :write-lines, :lifecycle/calls :onyx.plugin.sql/write-rows-calls} {:lifecycle/task :read-lines, :lifecycle/calls :onyx.plugin.kafka/read-messages-calls}) [{:lifecycle/task :write-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls1} {:lifecycle/task :read-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls1} {:lifecycle/task :write-lines, :lifecycle/calls :onyx.plugin.sql/write-rows-calls1} {:lifecycle/task :read-lines, :lifecycle/calls :onyx.plugin.kafka/read-messages-calls1}]))

lein test desdemona.query-test

lein test desdemona.tasks.kafka-test

lein test desdemona.workflows.sample-workflow-test

Ran 9 tests containing 20 assertions.
1 failures, 0 errors.
Tests failed.

Using is, the error shows up for the specific failure (using master)

Chris@juniper ~/C/desdemona (master)> lein test

lein test desdemona.functions.sample-functions-test

lein test desdemona.jobs.sample-job-test

lein test :only desdemona.jobs.sample-job-test/build-job-test

FAIL in (build-job-test) (sample_job_test.clj:27)
expected: (= lifecycles expected-lifecycles)
  actual: (not (= ({:lifecycle/task :write-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls} {:lifecycle/task :read-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls} {:lifecycle/task :write-lines, :lifecycle/calls :onyx.plugin.sql/write-rows-calls} {:lifecycle/task :read-lines, :lifecycle/calls :onyx.plugin.kafka/read-messages-calls}) [{:lifecycle/task :write-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls1} {:lifecycle/task :read-lines, :lifecycle/calls :desdemona.lifecycles.logging/log-calls1} {:lifecycle/task :write-lines, :lifecycle/calls :onyx.plugin.sql/write-rows-calls1} {:lifecycle/task :read-lines, :lifecycle/calls :onyx.plugin.kafka/read-messages-calls1}]))

lein test desdemona.query-test

lein test desdemona.tasks.kafka-test

lein test desdemona.workflows.sample-workflow-test

Ran 9 tests containing 20 assertions.
1 failures, 0 errors.