juji-io/editscript

throws when patch and equality test issue

huahaiy opened this issue · 9 comments

Hi, I finally got around testing this. Unfortunately, I'm running into some issues with this release]. When doing a trivial patch it throws.

(def edit-source {})
(def edit-dest {:x :hello-world})

"this throws"
(let [edits (editscript.edit/get-edits
              (editscript/diff edit-source edit-dest))]
  (editscript/patch edit-source (editscript.edit/edits->script edits))
  )

Throws with:

Execution error (AssertionError) at editscript.edit/edits->script (edit.cljc:198).
Assert failed: Not a vector of valid edits
(valid-edits? edits)

Also, I'm seeing some weird behavior with regard to equality:

(assert (= (editscript.edit/get-edits
                        (editscript/diff {} {}))
          (editscript.edit/get-edits
                        (editscript/diff {} {})))
  "this succeeds")

(def some-edits (editscript.edit/get-edits
                  (editscript/diff {} {})))

(assert (= some-edits (editscript.edit/get-edits
                        (editscript/diff {} {})))
  "this fails")

I created a full repro repo here: https://github.com/FreekPaans/editscript-repro/blob/master/src/editscript_test/core.clj. Also tested on Clojure 1.10.0, Java version was 11.

Originally posted by @FreekPaans in #8 (comment)

@FreekPaans Thank you for the report. It is because the condition of valid-edits? was too strict and some valid edits were considered invalid. I have released a new version 0.4.1 to fix this.

I was unable to reproduce the weird equality behavior though.

I can confirm that the valid-edits? problem doesn't occur anymore for the examples in the repro. I still need to test it in my actual project with some example data, I'll let you know when I get to that.

WRT equality: how did you test that? When I run it with lein the following happens:

lein run -m   editscript-test.core
Exception in thread "main" java.lang.AssertionError: Assert failed: this fails
(= some-edits (editscript.edit/get-edits (editscript/diff {} {}))), compiling:(editscript_test/core.clj:28:1)
        at clojure.lang.Compiler.load(Compiler.java:7526)
        at clojure.lang.RT.loadResourceScript(RT.java:379)
        at clojure.lang.RT.loadResourceScript(RT.java:370)
        at clojure.lang.RT.load(RT.java:460)
        at clojure.lang.RT.load(RT.java:426)
        at clojure.core$load$fn__6548.invoke(core.clj:6046)
        at clojure.core$load.invokeStatic(core.clj:6045)
        at clojure.core$load.doInvoke(core.clj:6029)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5848)
        at clojure.core$load_one.invoke(core.clj:5843)
        at clojure.core$load_lib$fn__6493.invoke(core.clj:5888)
        at clojure.core$load_lib.invokeStatic(core.clj:5887)
        at clojure.core$load_lib.doInvoke(core.clj:5868)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:659)
        at clojure.core$load_libs.invokeStatic(core.clj:5925)
        at clojure.core$load_libs.doInvoke(core.clj:5909)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:659)
        at clojure.core$require.invokeStatic(core.clj:5947)
        at clojure.core$require.doInvoke(core.clj:5947)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at user$eval149$fn__153.invoke(form-init14102477012176057333.clj:1)
        at user$eval149.invokeStatic(form-init14102477012176057333.clj:1)
        at user$eval149.invoke(form-init14102477012176057333.clj:1)
        at clojure.lang.Compiler.eval(Compiler.java:7062)
        at clojure.lang.Compiler.eval(Compiler.java:7052)
        at clojure.lang.Compiler.load(Compiler.java:7514)
        at clojure.lang.Compiler.loadFile(Compiler.java:7452)
        at clojure.main$load_script.invokeStatic(main.clj:278)
        at clojure.main$init_opt.invokeStatic(main.clj:280)
        at clojure.main$init_opt.invoke(main.clj:280)
        at clojure.main$initialize.invokeStatic(main.clj:311)
        at clojure.main$null_opt.invokeStatic(main.clj:345)
        at clojure.main$null_opt.invoke(main.clj:342)
        at clojure.main$main.invokeStatic(main.clj:424)
        at clojure.main$main.doInvoke(main.clj:387)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:702)
        at clojure.main.main(main.java:37)
Caused by: java.lang.AssertionError: Assert failed: this fails
(= some-edits (editscript.edit/get-edits (editscript/diff {} {})))
        at editscript_test.core$eval1307.invokeStatic(core.clj:28)
        at editscript_test.core$eval1307.invoke(core.clj:28)
        at clojure.lang.Compiler.eval(Compiler.java:7062)
        at clojure.lang.Compiler.load(Compiler.java:7514)
        ... 40 more```

@FreekPaans editscript/diff should be editscript.core/diff

@FreekPaans I think it is a Clojure bug in version 1.9.0. This equality weirdness only appears on Clojure 1.9.0. The latest Clojure releases, 1.10.x, does not have this problem.

@FreekPaans I think it is a Clojure bug in version 1.9.0. This equality weirdness only appears on Clojure 1.9.0. The latest Clojure releases, 1.10.x, does not have this problem.

Wow, weird. Can confirm it doesn't occur on 1.10.0 and 1.10.1. Any ideas what fixed this in 1.10? Quickly went through the release notes, but didn't see anything that looked like it could be it.

My guess would be the fixes in 4.1 Collections. For when the problem occurs in 1.9.0, the diff generated is wrong, [[[] :r {}]] instead of [].

Release 0.4.2 to accommodate Clojure 1.9.0.