riemann/riemann

Use `clojure.test/deftest` with `use-fixtures` instead of `riemann.test/deftest`

Opened this issue · 11 comments

andrusieczko commented

Hi,

There a couple problems with riemann.test/deftest:

  1. overriding deftest is quite confusing especially for people that are not familiar with Clojure
  2. it's impossible to run the test from an IDE like Cursive (in IntelliJ)
  3. while clojure.test/deftest is well understood by the IDE (Cursive), riemann.test/deftest is not
  4. it relies on some state that is defined in riemann.bin which is not obvious

I would like to propose a solution for this problem.

In the PR I provided (#778) I created a fixture that allows you to run the tests from an IDE using clojure.test/deftest.

The only one requirement is to set the environmental variable config what whatever you'd pass when running riemann test riemann.config.

Tested with changed, stated-changed, stable, runs and moving-event-window.

The example test could look like this:

(ns my-namespace
  (:require [clojure.test :refer :all]
            [riemann.test-runner :as test-runner]))

(use-fixtures :once test-runner/set-up-fixture)

(deftest some-test
  ; your test
  )

(deftest some-other-test
  ; your test
  )

Pull request: #778

jamtur01 commented

I'm going to defer to @pyr or @aphyr here.

aphyr commented

I don't fully understand what's going on here. I can see an argument for refactoring the riemann.test macros for establishing a test ns to do less magic overriding and use fixtures instead, but I don't know about the environment variable and all that--it feels like this might be specific to your particular dev environment, and I'd like to keep the stuff in Riemann itself broadly applicable for all users. Might be a good third-party library though?

andrusieczko commented

Hi,

I think I understand your concerns.

However, can you describe the recommended way of writing the rules and tests?

What we have in the project is the following:

  • many namespaces with rules
  • corresponding namespaces with tests for these rules
  • one main file that's including the rule namespaces

Now, I'm not sure how you normally develop in Clojure, but whenever I write a test, I like to run this one, not the whole suite.

I'm using Cursive for this (but this should be IDE agnostic).
When you have your REPL started, it just requires the namespace and runs the test using standard clojure.test.
I believe other environments do the same thing.

When deftest is stateful, it's impossible to do that.

The current way of running the tests is from the command line.
I don't like this solution for two reasons:

  • you need to start up the REPL every time (takes time, switching the context)
  • you can't run a particular test

Long story short - can you tell me a way to develop a riemann configuration project in a way where I am able to run the tests one by one?

  • 👍1
mcorbin commented

I agree with @andrusieczko, it would be nice to be able to launch tests from the repl.

andrusieczko commented

I'm not claiming that the implementation provided by me is the best solution :)
If I could I would get rid of all state from riemann but I simply didn't want to mess up the whole codebase :)

Anyway, as @mcorbin said, it's all about launching tests from the repl.
Maybe you have a better way to do it? :)

aphyr commented

Looking more at riemann.test... I think you can express both the current test system and your repl-friendly workflow with the same code. I suggest moving the functions you've written into riemann.test itself, deleting riemann.test/deftest, and modifying riemann.test/tests to set up fixtures instead. Should be API-compatible with the current test system, and then the only bit of custom code you'll need will be for loading the config file. Sound workable?

mcorbin commented

btw, i tried to work in this issue.
The fixture cannot be defined in riemann.test because of cyclic dependencies. I wrote this fixture in riemann.bin :

(defn test-fixture
  "Establishes a fresh time context and a fresh
  set of tap results for the duration of the body"
  [f]
  (test/with-test-env
    (riemann.config/validate-config @config-file)
    (riemann.time/reset-tasks!)
    (riemann.config/clear!)
    (riemann.config/include @config-file)
    (binding [test/*streams* (:streams @config/next-core)
              test/*results* (test/fresh-results @test/*taps*)]
      (riemann.time.controlled/with-controlled-time!
        (riemann.time.controlled/reset-time!)
        (f)))))

I removed the custom deftest in riemann.test and the binding clause in the test command in riemann.bin, and added the fixture in the (tests) macro :

'(use-fixtures :once riemann.bin/test-fixture)

It seems to work when i launch "riemann test riemann.config" but not when i launch tests using the repl (i have several NullPointerException, i think the reload in the fixture cause issues). I don't have time to investigate right now ;)

andrusieczko commented

I'll take a look at that during this weekend, thanks for your contribution!

andrusieczko commented

Hi @mcorbin,

Can you tell me how exactly did you try to run the tests from the REPL and what kind of NPE did you get? And what do you mean by "reload"?

Maybe we could use clojure.tools.namespace.repl/refresh? :)

And did you try with my fixture?

I'm trying to figure out why my solution was not working for you.

Thanks,
Karol

mcorbin commented

Hi,

I tested your fixture (forcing env-config to "/etc/riemann/riemann.config").

I have this code in a test namespace, working in Riemann 0.2.12 when in launch riemann test /etc/riemann/riemann.config

(def foo-stream
  (where (service "foo")
    (riemann.test/tap :foo)))

(riemann.test/tests
 (deftest foo-test
   (let [result (inject! [foo.foo-test/foo-stream] [{:service "foo"
                                                     :host "bar"}])]
     (is (= (:foo result) [{:service "foo"
                            :host "bar"}])))))

I added your fixture in riemann.bin, removed deftest in riemann.test, added

'(use-fixtures :once riemann.bin/set-up-fixture)`

In the tests macro. riemann test /etc/riemann/riemann.config seems to work fine.

With this configuration in my test ns:

(def foo-stream
  (where (service "foo")
    (riemann.test/tap :foo)))

(deftest foo-test
  (let [result (riemann.test/inject! [foo.foo-test/foo-stream] [{:service "foo"
                                                    :host "bar"}])]
    (is (= (:foo result) [{:service "foo"
                           :host "bar"}]))))

so without calling the fixture, riemann test /etc/riemann/riemann.config works. If i add (use-fixtures :once riemann.bin/set-up-fixture) in my configuration, i have :

WARN [2017-04-02 22:44:34,973] main - foo.foo-test - riemann.test$tap_stream$stream__11379@37606fee threw
java.lang.NullPointerException: null
	at clojure.core$swap_BANG_.invokeStatic(core.clj:2261)
	at clojure.core$swap_BANG_.invoke(core.clj:2253)
	at riemann.test$tap_stream$stream__11379.invoke(test.clj:41)
	at foo.foo_test$fn__38$stream__39$fn__44.invoke(foo_test.clj:12)
	at foo.foo_test$fn__38$stream__39.invoke(foo_test.clj:12)
	at riemann.test$inject_BANG_$fn__11390.invoke(test.clj:144)
	at clojure.core$with_redefs_fn.invokeStatic(core.clj:7216)
	at clojure.core$with_redefs_fn.invoke(core.clj:7200)
	at riemann.test$inject_BANG_.invokeStatic(test.clj:135)
	at riemann.test$inject_BANG_.invoke(test.clj:124)
	at foo.foo_test$fn__85.invokeStatic(foo_test.clj:16)
	at foo.foo_test$fn__85.invoke(foo_test.clj:15)
	at clojure.test$test_var$fn__7983.invoke(test.clj:716)
	at clojure.test$test_var.invokeStatic(test.clj:716)
	at clojure.test$test_var.invoke(test.clj:707)
	at clojure.test$test_vars$fn__8005$fn__8010.invoke(test.clj:734)
	at clojure.test$default_fixture.invokeStatic(test.clj:686)
	at clojure.test$default_fixture.invoke(test.clj:682)
	at clojure.test$test_vars$fn__8005.invoke(test.clj:734)
	at riemann.bin$run_test$fn__12574.invoke(bin.clj:129)
	at clojure.core$with_redefs_fn.invokeStatic(core.clj:7216)
	at clojure.core$with_redefs_fn.invoke(core.clj:7200)
	at riemann.bin$run_test.invokeStatic(bin.clj:126)
	at riemann.bin$run_test.invoke(bin.clj:123)
	at riemann.bin$set_up_for_single_test.invokeStatic(bin.clj:140)
	at riemann.bin$set_up_for_single_test.invoke(bin.clj:131)
	at riemann.bin$set_up_fixture.invokeStatic(bin.clj:163)
	at riemann.bin$set_up_fixture.invoke(bin.clj:142)
	at clojure.test$compose_fixtures$fn__7977$fn__7978.invoke(test.clj:693)
	at clojure.test$default_fixture.invokeStatic(test.clj:686)
	at clojure.test$default_fixture.invoke(test.clj:682)
	at clojure.test$compose_fixtures$fn__7977.invoke(test.clj:693)
	at clojure.test$test_vars.invokeStatic(test.clj:730)
	at clojure.test$test_all_vars.invokeStatic(test.clj:736)
	at clojure.test$test_ns.invokeStatic(test.clj:757)
	at clojure.test$test_ns.invoke(test.clj:742)
	at clojure.core$map$fn__4785.invoke(core.clj:2644)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.RT.boundedLength(RT.java:1749)
	at clojure.lang.RestFn.applyTo(RestFn.java:130)
	at clojure.core$apply.invokeStatic(core.clj:648)
	at clojure.test$run_tests.invokeStatic(test.clj:767)
	at clojure.test$run_tests.doInvoke(test.clj:767)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:646)
	at clojure.test$run_all_tests.invokeStatic(test.clj:779)
	at clojure.test$run_all_tests.invoke(test.clj:779)
	at riemann.bin$_main$fn__12565.invoke(bin.clj:111)
	at riemann.bin$_main.invokeStatic(bin.clj:106)
	at riemann.bin$_main.doInvoke(bin.clj:85)
	at clojure.lang.RestFn.invoke(RestFn.java:425)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at riemann.bin.main(Unknown Source)

FAIL in (foo-test) (foo_test.clj:18)
expected: (= (:foo result) [{:service "foo", :host "bar"}])
  actual: (not (= nil [{:service "foo", :host "bar"}]))

If i start Riemann, connect using cider to repl, and launch (clojure.test/run-tests) in my test namespace, with the second configuration (without use-fixture), i have :

Testing foo.foo-test

ERROR in (foo-test) (core.clj:2208)
Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.NullPointerException: null
 at clojure.core$deref_future.invokeStatic (core.clj:2208)
    clojure.core$deref.invokeStatic (core.clj:2228)
    clojure.core$deref.invoke (core.clj:2214)
    riemann.test$inject_BANG_.invokeStatic (test.clj:133)
    riemann.test$inject_BANG_.invoke (test.clj:124)
    foo.foo_test$fn__83.invokeStatic (foo_test.clj:14)
    foo.foo_test/fn (foo_test.clj:13)
    clojure.test$test_var$fn__7983.invoke (test.clj:716)
    clojure.test$test_var.invokeStatic (test.clj:716)
    clojure.test$test_var.invoke (test.clj:707)
    clojure.test$test_vars$fn__8005$fn__8010.invoke (test.clj:734)
    clojure.test$default_fixture.invokeStatic (test.clj:686)
    clojure.test$default_fixture.invoke (test.clj:682)
    clojure.test$test_vars$fn__8005.invoke (test.clj:734)
    clojure.test$default_fixture.invokeStatic (test.clj:686)
    clojure.test$default_fixture.invoke (test.clj:682)
    clojure.test$test_vars.invokeStatic (test.clj:730)
    clojure.test$test_all_vars.invokeStatic (test.clj:736)
    clojure.test$test_ns.invokeStatic (test.clj:757)
    clojure.test$test_ns.invoke (test.clj:742)
    clojure.core$map$fn__4785.invoke (core.clj:2646)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.boundedLength (RT.java:1749)
    clojure.lang.RestFn.applyTo (RestFn.java:130)
    clojure.core$apply.invokeStatic (core.clj:648)
    clojure.test$run_tests.invokeStatic (test.clj:767)
    clojure.test$run_tests.doInvoke (test.clj:767)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    clojure.test$run_tests.invokeStatic (test.clj:772)
    clojure.test$run_tests.invoke (test.clj:767)
    foo.foo_test$eval104.invokeStatic (riemann.config:88)
    foo.foo_test$eval104.invoke (riemann.config:88)
    clojure.lang.Compiler.eval (Compiler.java:6927)
    clojure.lang.Compiler.eval (Compiler.java:6890)
    clojure.core$eval.invokeStatic (core.clj:3105)
    clojure.core$eval.invoke (core.clj:3101)
    clojure.main$repl$read_eval_print__7408$fn__7411.invoke (main.clj:240)
    clojure.main$repl$read_eval_print__7408.invoke (main.clj:240)
    clojure.main$repl$fn__7417.invoke (main.clj:258)
    clojure.main$repl.invokeStatic (main.clj:258)
    clojure.main$repl.doInvoke (main.clj:174)
    clojure.lang.RestFn.invoke (RestFn.java:1523)
    clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__10932.invoke (interruptible_eval.clj:87)
    clojure.lang.AFn.applyToHelper (AFn.java:152)
    clojure.lang.AFn.applyTo (AFn.java:144)
    clojure.core$apply.invokeStatic (core.clj:646)
    clojure.core$with_bindings_STAR_.invokeStatic (core.clj:1881)
    clojure.core$with_bindings_STAR_.doInvoke (core.clj:1881)
    clojure.lang.RestFn.invoke (RestFn.java:425)
    clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic (interruptible_eval.clj:85)
    clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke (interruptible_eval.clj:55)
    clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__10977$fn__10980.invoke (interruptible_eval.clj:222)
    clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__10972.invoke (interruptible_eval.clj:190)
    clojure.lang.AFn.run (AFn.java:22)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1142)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:617)
    java.lang.Thread.run (Thread.java:745)

Ran 1 tests containing 1 assertions.
0 failures, 1 errors.
{:test 1, :pass 0, :fail 0, :error 1, :type :summary}

If i add use-fixture, i don't have exceptions but i have :

Testing foo.foo-test
foo  {}

FAIL in (foo-test) (foo_test.clj:16)
expected: (= (:foo result) [{:service "foo", :host "bar"}])
  actual: (not (= nil [{:service "foo", :host "bar"}]))

Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
{:test 1, :pass 0, :fail 1, :error 0, :type :summary}

I think some variables (like results or taps) are not correctly initialized.

pgilad commented

@andrusieczko @mcorbin were you ever able to run riemann tests using repl?