taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 130 forks source link

To migrate tests from expectations to clojure.test #191

Closed firesofmay closed 7 years ago

firesofmay commented 7 years ago

Hi, In project.clj I see this comment "TODO Migrate expectations->clojure.test" [1] Is this something you are still interested in doing? I'd be happy to do this and send a pull request. Just want to confirm.

Thanks.

[1] : https://github.com/ptaoussanis/carmine/blob/master/project.clj#L30

ptaoussanis commented 7 years ago

Hi Mayank, sure - I'd be up for a PR! That'd be great, thank you!

firesofmay commented 7 years ago

@ptaoussanis Great! I was looking at run-tests file. It says it Usage: ./run-tests <env-file> [--auto] But I don't see any env-file inside the library or in test dir. What should be passed here?

ptaoussanis commented 7 years ago

That's only used for the Tundra tests.

Ideally, the Tundra tests could be updated to switch from an S3 store to a disk store.

In that case, we can also drop the env-file stuff.

You're welcome to either make that switch yourself if you feel like it, or you can just migrate the tests as-is - just note that your Tundra tests will fail unless there's valid S3 credentials in the env-file.

Cheers! :-)

firesofmay commented 7 years ago

@ptaoussanis Awesome. Will get back if I have anymore questions. Thanks for the quick reply.

firesofmay commented 7 years ago

@ptaoussanis I have migrated taoensso.carmine.locks. So I ran the test and it passed. But I am getting a failure in running taoensso.carmine.tests.main test. I am running redis 3.2.8 version on Mac OSX.

$ lein expectations taoensso.carmine.tests.main

failure in (main.clj:410) : taoensso.carmine.tests.main
(expect
 "Oh noes!"
 (wcar
  {}
  (->>
   (car/return (Exception. "boo"))
   (car/parse
    (->
     (fn*
      [p1__11944#]
      (if (instance? Exception p1__11944#) "Oh noes!" p1__11944#))
     (with-meta {:parse-exceptions? true}))))))

  act-msg: exception in actual: (wcar {} (->> (car/return (Exception. "boo")) (car/parse (-> (fn* [p1__11944#] (if (instance? Exception p1__11944#) "Oh noes!" p1__11944#)) (with-meta {:parse-exceptions? true})))))
    threw: class java.lang.Exception - boo
           on (main.clj:410)
           taoensso.carmine.protocol$_with_replies$invokeStatic (protocol.clj:340)
           on (protocol.clj:328)
           taoensso.carmine.tests.main$expect1025502580$invokeStatic (main.clj:410)
           user$eval13057$invokeStatic (form-init8347337024714921786.clj:1)
           on (form-init8347337024714921786.clj:1)

Benching (this can take some time)
----------------------------------

Lap 1/1...
{:wcar 34, :ping 502, :set 437, :get 397, :roundtrip 467, :ping-pipelined 2033}

Done! (Time for cake?)

Ran 118 tests containing 118 assertions in 8782 msecs
0 failures, 1 errors.

Is there a setup issue? This shouldn't have failed.

More info:

$ java -version
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)
$ lein version
Leiningen 2.7.1 on Java 1.8.0_45 Java HotSpot(TM) 64-Bit Server VM
firesofmay commented 7 years ago

Also tests for taoensso.carmine.tests.message-queue are failing for 1.9 profile

$ lein with-profile +1.9 test taoensso.carmine.tests.message-queue
lein test taoensso.carmine.tests.message-queue
Running message queue tests

lein test :only taoensso.carmine.tests.message-queue/tests-1

FAIL in (tests-1) (message_queue.clj:41)
expected: ["mid1" :msg1 1]
  actual: "eoq-backoff"
    diff: - ["mid1" :msg1 1]
          + "eoq-backoff"

lein test :only taoensso.carmine.tests.message-queue/tests-1

FAIL in (tests-1) (message_queue.clj:42)
expected: :locked
  actual: :queued
    diff: - :locked
          + :queued

lein test :only taoensso.carmine.tests.message-queue/tests-1

FAIL in (tests-1) (message_queue.clj:43)
expected: "eoq-backoff"
  actual: ["mid1" :msg1 1]
    diff: - "eoq-backoff"
          + ["mid1" :msg1 1]

lein test :only taoensso.carmine.tests.message-queue/tests-1

FAIL in (tests-1) (message_queue.clj:44)
expected: nil
  actual: "eoq-backoff"
    diff: + "eoq-backoff"

Ran 7 tests containing 58 assertions.
4 failures, 0 errors.
Error encountered performing task 'test' with profile(s): 'base,system,user,provided,dev,1.9'
Tests failed.
firesofmay commented 7 years ago

One more issue I have noticed is if I run all tests for all profiles together for test taoensso.carmine.tests.message-queue it hangs on 1.5 profile for some reason.

You can see the issues on the travis Build that I ran: https://travis-ci.org/firesofmay/carmine/jobs/212246503

firesofmay commented 7 years ago

@ptaoussanis I've shared a pull request to fix failing tests in taoensso.carmine.tests.main ns in #192 Will check other failing tests in sometime. Thanks

ptaoussanis commented 7 years ago

Also tests for taoensso.carmine.tests.message-queue are failing for 1.9 profile

Thanks for sharing the output. This looks like a timing issue to me; the message queue tests have always been a little more fragile than I'd like. Happy to debug these when I merge since I can take the opportunity to maybe tweak the tests a little.

ptaoussanis commented 7 years ago

But I am getting a failure in running taoensso.carmine.tests.main test

Am I correct in assuming that your fix in #192 resolves this?

firesofmay commented 7 years ago

@ptaoussanis Yes that pull request fixes taoensso.carmine.tests.main failing tests. And yes message queue tests are passing now. I'll just go ahead and refactor the tests