clj-json-patch icon indicating copy to clipboard operation
clj-json-patch copied to clipboard

Missed some changes to ClojureScript

Open ILoveHubGit opened this issue 5 years ago • 4 comments

Sorry but with my previous pull request I broke your library. Here an update. I think that I was somewhat to eager in solving the conflicts.

Hope I did a better job this time.

clj-json-patch.core=> (use 'midje.repl)
Run `(doc midje)` for Midje usage.
Run `(doc midje-repl)` for descriptions of Midje repl functions.
nil
clj-json-patch.core=> (autotest)

======================================================================
Loading (clj-json-patch.util clj-json-patch.rfc6901-test clj-json-patch.core clj-json-patch.util-test clj-json-patch.core-test)
nil
All checks (103) succeeded.
[Completed at 23:34:33]
true

I now ran the tests within a REPL, because on my PC lein test does not seem to work as no test is run then.

ILoveHubGit avatar Mar 09 '20 22:03 ILoveHubGit

Thanks for the library updates!

Interesting, because the unit tests passed when I ran them from your code.

Run lein midje. I guess I never documented that in the README.md.

Taking a look now.

daviddpark avatar Mar 19 '20 01:03 daviddpark

Correct. For me the unit tests worked also. But lein midje only checks the Clojure implementation and not the ClojureScript implementation. I ran into this error when I tried to use the new version 0.1.7

I'm working on another update so it is possible to run all tests also against ClojureScript. So you can expect another pull request in the near future :-)

ILoveHubGit avatar Mar 19 '20 12:03 ILoveHubGit

Sorry it took some extra time to solve the problems I ran into. I added a test in which a vector contains multiple maps of which two are changed. This was the JSON part that went wrong in my previous tests. To solve this I changed the order of the condition in the function diff-vecs. I didn't add the promised ClojureScript automatic tests. But I tested them all in a test application I wrote (https://github.com/ILoveHubGit/testjsonpatch)

Thanks for your patience.

ILoveHubGit avatar Aug 05 '20 19:08 ILoveHubGit

At last I succeeded in adding also the unit tests for ClojureScript. I only implemented the tests that were in core_test.clj because the function tested in the other functions are already tested via Clojure (lein midje).

I would love to see this merged with your library.

ILoveHubGit avatar Jun 15 '21 20:06 ILoveHubGit