marick / Midje

Midje provides a migration path from clojure.test to a more flexible, readable, abstract, and gracious style of testing
MIT License
1.69k stars 129 forks source link

Midje doesn't compare values with ##NaN #432

Closed nukep closed 6 years ago

nukep commented 6 years ago

How to reproduce:

(fact "NaN compares to itself"
  ##NaN => ##NaN

  {:value ##NaN} => {:value ##NaN})

Outputs:

FAIL "NaN compares to itself" at (mytest.clj:40)
Expected:
##NaN
Actual:
##NaN

FAIL "NaN compares to itself" at (mytest.clj:41)
Expected:
{:value ##NaN}
Actual:
{:value ##NaN}
Diffs: in [:value] expected ##NaN, was ##NaN
nil

This is especially a pain if you use generative testing, e.g. with gen/any

(:require [midje.sweet :refer :all]
          [midje.experimental :refer [for-all]
          [clojure.test.check.generators :as gen])

(for-all
  [x gen/any]
  {:num-tests 1000}
  (fact "all values compare to itself"
    x => x))

Output:

FAIL "all values compare to itself" at (mytest.clj:34)
Expected:
#{{##NaN 0}}
Actual:
#{{##NaN 0}}

quick-check seed:
1514579126990

quick-check shrunken failing values:
{x #{{##NaN 0}}}
nil

I'm not really sure what the best way to approach this is. I'm aware that NaN is not meant to compare to anything, so in most cases, it makes sense that (= ##NaN x) always returns false. With that said, I don't think it makes much sense to uphold the property in unit tests. Maybe we can treat ##NaN specially? I noticed that clojure.test passes the test:

(:use clojure.test)

(deftest mytest
  (testing "NaN compares to itself"
    (is (= ##NaN ##NaN))))

~My workaround for the generative testing case is to replace gen/any with (gen/such-that #(= %1 %1) gen/any). e.g.~ EDIT: doesn't work

(for-all
  [x (gen/such-that #(= %1 %1) gen/any)]
  {:num-tests 1000}
  (fact "all values compare to itself"
    x => x))
nukep commented 6 years ago

I apologize - my initial workaround doesn't actually work. I discovered that a function that compares NaN returns true, just not when it's evaluated outside a function. This is really strange.

(defn equals-itself [x] (= x x))

(facts "test"
  (equals-itself [1]) => true
  (equals-itself ##NaN) => true
  (= ##NaN ##NaN) => false)
philomates commented 6 years ago

Thanks for reporting this, I was able to reproduce it locally. I'll take a crack at addressing it in the next few days, though it might be sorta subtle to address because it will touch core equality checking code.

If this is blocking you in the meantime, I would suggest trying to adapt the generator to not generate ##NaN values.

To elaborate the cause of the issue, as I understand it: This arises due to ##NaN being an object, so

(= ##NaN ##NaN) ;; is false

but

(.equals ##NaN ##NaN) ;; is true

and then nested structures become a bit tricky:

(.equals {##NaN 1} {##NaN 1}) ;; is false
(.equals {:a ##NaN} {:a ##NaN}) ;; is true (wtf)
philomates commented 6 years ago

Upon reading about this a little more, the floating point specification for NaN states that it shouldn't be equal to itself (wikipedia), so this behavior is expected. With some searching, there are a bunch of sources about this behavior in Java to be found.

I'm going to close out this issue because the midje checker is in line with the floating-point spec as far as I can tell.

Lastly, I'm not entirely sure why equals-itself makes this comparison result in true. Would be curious to hear the answer if anyone has it :)