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.

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

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

Open
andrusieczko opened this issue Feb 18, 2017 · 11 comments

Comments

@andrusieczko
Copy link
Contributor

andrusieczko commented Feb 18, 2017

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

andrusieczko added a commit to andrusieczko/riemann that referenced this issue Feb 18, 2017
@jamtur01
Copy link
Member

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

@aphyr
Copy link
Collaborator

aphyr commented Feb 18, 2017

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
Copy link
Contributor Author

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?

@mcorbin
Copy link
Contributor

mcorbin commented Feb 19, 2017

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

@andrusieczko
Copy link
Contributor Author

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
Copy link
Collaborator

aphyr commented Feb 20, 2017

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
Copy link
Contributor

mcorbin commented Mar 30, 2017

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
Copy link
Contributor Author

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

@andrusieczko
Copy link
Contributor Author

andrusieczko commented Apr 2, 2017

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
Copy link
Contributor

mcorbin commented Apr 2, 2017

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
Copy link
Contributor

pgilad commented Dec 12, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants