Re: Possible Bug In clojure.zip/remove
Frantisek Sodomka wrote: On Mar 19, 12:58 pm, Jason Sankey ja...@zutubi.com wrote: Also, is there somewhere I can contribute test cases for this to prevent a future regression? Tests for clojure.zip can from now on go to test-clojure.clojure-zip: http://code.google.com/p/clojure-contrib/source/browse/trunk/src/clojure/contrib/test_clojure/clojure_zip.clj Thanks, I plan on posting my CA today and can then look at adding a few cases. On Mar 20, 1:24 pm, Jason Sankey ja...@zutubi.com wrote: 3) Test-is reports results for all (is ...) expressions separately, but there is no good way to name them separately. At the moment I am reporting on each (is ...) as a separate test case by using the test name plus line number, but this is clunky. I think combining the results so one deftest == one JUnit report test case would be more sensible. If there are any troubles with test-is, you can consult Stuart Sierra (the author of test-is). Although it may have sounded that way, I didn't intend to suggest that the default test-is behaviour was wrong, I presumed it was intended that way, and I'm sure it has advantages. For reporting the results in a CI server, though, and in particular when tracking the state of the same test across multiple builds, having each (is ...) as a separate case does not make sense. My own reporting code can handle this be combining the results of multiple callbacks to (report ...) into a single test case result, which is what I plan to do. Cheers, Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
On Mar 19, 12:58 pm, Jason Sankey ja...@zutubi.com wrote: Also, is there somewhere I can contribute test cases for this to prevent a future regression? Tests for clojure.zip can from now on go to test-clojure.clojure-zip: http://code.google.com/p/clojure-contrib/source/browse/trunk/src/clojure/contrib/test_clojure/clojure_zip.clj On Mar 20, 1:24 pm, Jason Sankey ja...@zutubi.com wrote: 3) Test-is reports results for all (is ...) expressions separately, but there is no good way to name them separately. At the moment I am reporting on each (is ...) as a separate test case by using the test name plus line number, but this is clunky. I think combining the results so one deftest == one JUnit report test case would be more sensible. If there are any troubles with test-is, you can consult Stuart Sierra (the author of test-is). Greetings, Frantisek --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
On Mar 19, 11:37 pm, Jason Sankey ja...@zutubi.com wrote: I pretty much have it working for the test-clojure suite now, although I'm sure the code could use review by a more experienced eye. I've been looking at adding the other two top-level suites (test-contrib and datalog) too, but their test clojure scripts are structured slightly differently such that requiring them leads them to run their tests immediately. It would be nice if some convention could be established for a top-level suite to be defined/exposed, and in such a way that I could wrap them with my script which defines special reporting methods to spit out XML. Nice! I took the liberty and changed the test-contrib and the datalog top- level tests to match test-clojure. Let me know if that helps or if something is broken :) Frantisek --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
Hi Frantisek, Frantisek Sodomka wrote: On Mar 19, 11:37 pm, Jason Sankey ja...@zutubi.com wrote: I pretty much have it working for the test-clojure suite now, although I'm sure the code could use review by a more experienced eye. I've been looking at adding the other two top-level suites (test-contrib and datalog) too, but their test clojure scripts are structured slightly differently such that requiring them leads them to run their tests immediately. It would be nice if some convention could be established for a top-level suite to be defined/exposed, and in such a way that I could wrap them with my script which defines special reporting methods to spit out XML. Nice! I took the liberty and changed the test-contrib and the datalog top- level tests to match test-clojure. Let me know if that helps or if something is broken :) Sweet, this is perfect. I have now cobbled together a proof-of-concept on my build server using my code to run the three top-level suites and produce JUnit-compatible reports. I have kept this separate at the moment from the primary clojure-contrib project as there are issues I need to sort out: 1) At the moment I'm just copying in my extra code and an extra build file to Make Things Work. I need a nicer way to layer this on. 2) A few things are hard coded which need not be. 3) Test-is reports results for all (is ...) expressions separately, but there is no good way to name them separately. At the moment I am reporting on each (is ...) as a separate test case by using the test name plus line number, but this is clunky. I think combining the results so one deftest == one JUnit report test case would be more sensible. 4) I'm still new to clojure, so I'm sure my code is far from optimal. In any case, it's nice to have come this far already, and perhaps the hackish reports can even start to be useful. You can take a look at an example build here: http://pulse.zutubi.com/browse/projects/clojure-contrib-reports/builds/5/ ans see that it reports 801 tests, all passed ;). Cheers, Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Possible Bug In clojure.zip/remove
Hi All, I've been teaching myself clojure and in the process am working on a small program to manipulate an XML file. I figured this was a good chance to try clojure.zip, but I'm having some difficulty. Being a newbie I strongly suspected myself - but when I posed the question on IRC the conclusion was there appears to be a bug. I boiled it down to the following: (require ['clojure.xml :as 'xml] ['clojure.zip :as 'zip]) (defn strip [root tag] (loop [loc root] (if (zip/end? loc) (zip/root loc) (recur (zip/next (if (= (:tag (zip/node loc)) tag) (zip/remove loc) loc)) (def data ?xml version=\1.0\?rootfoo/bar//root) (def xml-tree (xml/parse (java.io.ByteArrayInputStream. (.getBytes data (def zip-root (zip/xml-zip xml-tree)) (xml/emit (strip zip-root :foo)) (xml/emit (strip zip-root :bar)) The first call to strip :foo works as expected, but the following call to strip :bar throws an NPE within zip/remove. On further inspection we came clojure/zip.clj:234: (if (branch? loc) (recur (- loc down rightmost)) loc)) According to the docs, branch? can return true for a loc that does not currently have any children, so the suspicion is that it is an insufficient test before moving down as happens in the line below. I can fix this for my simple test cases by replacing the above with: (if (and (branch? loc) (not (empty? (children loc (recur (- loc down rightmost)) loc)) Being new to both clojure and zip I admit there's still a good chance that I have no idea what I'm talking about :). Cheers, Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
Hi Christophe, On Mar 19, 11:22 am, Christophe Grand christo...@cgrand.net wrote: Jason Sankey a écrit : (if (and (branch? loc) (not (empty? (children loc (recur (- loc down rightmost)) loc)) Being new to both clojure and zip I admit there's still a good chance that I have no idea what I'm talking about :). No you are right and there's the same bug in prev. Here is a patch that fixes both bugs. (Rich, can I open an issue?) Thanks for the reply, and a patch which is (not surprisingly) far more elegant than my hack. Maybe this little bit of logic should be extract to its own function (alarms go off when an identical bug shows up in two places)? Also, is there somewhere I can contribute test cases for this to prevent a future regression? Btw, (not (empty? coll)) is not idiomatic, (seq coll) is the clojure way to test for non-emptiness. Thanks, I appreciate the tip. Thanks for the report, No problem, Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
On Mar 19, 7:22 am, Christophe Grand christo...@cgrand.net wrote: Jason Sankey a écrit : (if (and (branch? loc) (not (empty? (children loc (recur (- loc down rightmost)) loc)) Being new to both clojure and zip I admit there's still a good chance that I have no idea what I'm talking about :). No you are right and there's the same bug in prev. Here is a patch that fixes both bugs. (Rich, can I open an issue?) Patch applied - svn 1335 - thanks! Rich --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
On Mar 19, 12:58 pm, Jason Sankey ja...@zutubi.com wrote: Also, is there somewhere I can contribute test cases for this to prevent a future regression? In order to contribute, you must fill-in and send The Contributor Agreement (CA) to Rich Hickey: http://clojure.org/contributing Tests for clojure.core are currently in clojure-contrib in test- clojure: http://code.google.com/p/clojure-contrib/source/browse/#svn/trunk/src/clojure/contrib/test_clojure There is no place yet where to put clojure.xml tests, but it shouldn't be hard to create something for it. After Rich receives your CA, you can create an issue and post a patch for it: http://code.google.com/p/clojure-contrib/issues/list An example issue: http://code.google.com/p/clojure-contrib/issues/detail?id=15 Your help with testing Clojure is very welcome! Thank you, Frantisek --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
Re: Possible Bug In clojure.zip/remove
Hi Frantisek, Frantisek Sodomka wrote: On Mar 19, 12:58 pm, Jason Sankey ja...@zutubi.com wrote: Also, is there somewhere I can contribute test cases for this to prevent a future regression? In order to contribute, you must fill-in and send The Contributor Agreement (CA) to Rich Hickey: http://clojure.org/contributing Thanks, I have one printed out, and hope to mail it off at the start of next week. Tests for clojure.core are currently in clojure-contrib in test- clojure: http://code.google.com/p/clojure-contrib/source/browse/#svn/trunk/src/clojure/contrib/test_clojure There is no place yet where to put clojure.xml tests, but it shouldn't be hard to create something for it. After Rich receives your CA, you can create an issue and post a patch for it: http://code.google.com/p/clojure-contrib/issues/list An example issue: http://code.google.com/p/clojure-contrib/issues/detail?id=15 Your help with testing Clojure is very welcome! I appreciate the information, and will be happy to chip in with some tests when I am all CA'd up. In the mean time, as mentioned on IRC I am setting up another (unofficial) build server. I have it building both clojure and contrib already, and in the latter it runs the test suites. I would like to take this to the next step by generating reports from the tests which the server can read and thus report on the test results nicely in the UI. I am aiming to generate JUnit-compatible XML output, as this is the most widely-supported format by a variety of tools. I pretty much have it working for the test-clojure suite now, although I'm sure the code could use review by a more experienced eye. I've been looking at adding the other two top-level suites (test-contrib and datalog) too, but their test clojure scripts are structured slightly differently such that requiring them leads them to run their tests immediately. It would be nice if some convention could be established for a top-level suite to be defined/exposed, and in such a way that I could wrap them with my script which defines special reporting methods to spit out XML. Cheers, Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Clojure group. To post to this group, send email to clojure@googlegroups.com To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en -~--~~~~--~~--~--~---
