ninjudd / cake

A tasty build tool for Clojure.
clojure-cake.org
Eclipse Public License 1.0
171 stars 12 forks source link

test-ns-hook support #99

Closed rosejn closed 11 years ago

rosejn commented 13 years ago

This patch adds support for the test-ns-hook function that can optionally be defined with clojure.test to have control over the order that tests are run inside a namespace.

It would be nice if there were a way to query for the current cake version, or to use the source directly from a checked out git repo.

Cheers, Jeff

lancepantz commented 13 years ago

Hi Jeff- I just took a look at this patch. We definitely do need to support test-ns-hooks, but this patch breaks some of the other (probably undocumented) features of cake's test task.

You can optionally pass a namespace or qualified test name to the test task to run tests for a specific ns or a specific test. You can also add tags to a tests' metadata, then run test for a specific tag by specifying the tag as keyword argument to the test task.

In general, the ability to only run certain tests is colliding with the functionality offered by test-ns-hook. Justin and I had a conversation about how to address this, and came up with this-

 Given the test-ns-hook is defined
      when the test task is run with no options
           then all test nses should be run and the test-ns-hook should be obeyed
      when the test task is run specifying a ns
           then the specified ns should be run and the test-ns-hook should be obeyed
      when tests are tagged and tags are specified
           then the tagged tests should be run and test-ns-hook should be ignored (with a warning)
      when a specific test is specified
           then the specified test should be run and test-ns-hook should be ignored (with a warning)

 Given a test-ns-hook is not defined
      then the test task should work as it does now

I think that covers all of the permutations... does that make sense?

Thanks for you help! -lance

rosejn commented 13 years ago

Sending you a new pull request with these rules implemented now.

rosejn commented 13 years ago

oh whoops, didn't realize it automatically adds my new commit to the previous pull request... sweet.

lancepantz commented 13 years ago

Hey jeff. sorry for the delay! we're attempting to use git flow (http://nvie.com/posts/a-successful-git-branching-model/) so i rebased your commit into the develop branch. once we cut a new gem it will get pulled into master. thanks for the patch!

rosejn commented 13 years ago

Cool, thanks for the update. So the result of this strategy is that in the future I should create a separate branch for each patch?

lancepantz commented 13 years ago

when you're working in your fork it doesn't matter. but if you're committing to ninjudd's repo, then yes.

in general though, i think it's a good habit to get into.

ninjudd commented 13 years ago

Hey Jeff. I was using cake today and noticed that I was getting test-ns-hooks warnings whenever I ran a limited set of tests using something like cake test graph.reader. I reverted the relevant commits for now, though I probably should have just disabled the warnings.

If you have some time to take a look, great, otherwise, Lance or I will try to get to it next week sometime.