Skip to content
This repository has been archived by the owner on May 10, 2023. It is now read-only.

Change 'Run all tests' to only run those defined in the current project #145

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

moxaj
Copy link
Contributor

@moxaj moxaj commented Aug 15, 2016

No description provided.

@jasongilman
Copy link
Owner

This is a neat change. I made a similar change at the same time since I didn't know you were working it as well. I'll try out your additional changes.

@jasongilman
Copy link
Owner

The tests thing seems to work but I don't quite understand "how it works" so that makes me nervous to integrate it. Can you explain how it's limiting the tests to just the current project?

@moxaj
Copy link
Contributor Author

moxaj commented Aug 19, 2016

scan-all does all the magic, but i'm not familiar with its internals. Its return value is described at https://github.com/clojure/tools.namespace/blob/64e807c1888b494d04bb4dc96432d4d76aa0db54/src/main/clojure/clojure/tools/namespace/track.cljc#L116.

Also I pushed a new commit which fixes a minor issue; if a namespace had no other namespace required, then it also wasn't present in the dependencies of the tracker.

@carocad
Copy link
Contributor

carocad commented Aug 19, 2016

I was just wondering ... wouldn't it be easier to just pass a regular expression to the run-all-test function?
I mean, if we are in a clojure project and we can recognize the namespace, wouldn't it be something like #"root-ns.*" ?

According to the documentation, that's the official way to limit the tests: http://clojuredocs.org/clojure.test/run-all-tests

@moxaj
Copy link
Contributor Author

moxaj commented Aug 19, 2016

Sure, but:

  • you still have to find out what root-ns is
  • there might be more than one root-ns
  • what if a library you depend on uses the same root-ns? (unlikely, but possible)
  • (run-tests & namespaces) is just as official, the magic is finding out what namespaces is

@carocad
Copy link
Contributor

carocad commented Aug 19, 2016

you still have to find out what root-ns is

proto-repl already scan your file to extract its ns for evaluation. I don't think it should be that difficult to find the root-ns, considering that you just have to split the string by #"\." and get the first one.

there might be more than one root-ns

I have never seen that but I guess it is possible

what if a library you depend on uses the same root-ns? (unlikely, but possible)

Is that really possible? if you both use the same ns wouldn't you get in an egg/chicken dilemma when writing your project.clj?
(defproject rootns :dependencies [rootns "0.1.2"]
right? that would definitely be a circular dependency so I'm not sure if I got you right

(run-tests & namespaces) is just as official, the magic is finding out what namespaces is

Absolutely

One last thing to mention is that clojure.tools.namespace is a separate library, not part of clojure.core meaning that the user would need to add it to its dependencies before he can run all tests, as per: https://github.com/clojure/tools.namespace

@moxaj
Copy link
Contributor Author

moxaj commented Aug 19, 2016

there might be more than one root-ns

If you have multiple source directories, I think it is possible

what if a library you depend on uses the same root-ns? (unlikely, but possible)

By possible I meant i'm not sure if it's impossible :)

As for the last point, the current runAllTests already depends on refreshNamespaces, which reports a warning if you don't have tools.namespace as a dependency, so I don't think this is an issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants