probcomp / metaprob

An embedded language for probabilistic programming and meta-programming.
GNU General Public License v3.0
168 stars 17 forks source link

The test files exist but they don't get imported and used #150

Open bzinberg opened 5 years ago

bzinberg commented 5 years ago

Part 2

(require `metaprob.trace-test)
; Output:
;   FileNotFoundException Could not locate metaprob/trace_test__init.class or
;   metaprob/trace_test.clj on classpath. Please check that namespaces with
;   dashes use underscores in the Clojure file name.  clojure.lang.RT.load
;   (RT.java:463)

Also tried modifying the import statement to be relative to the repo root dir, didn't work (not that I really expected it to):

(require 'test.metaprob.trace-test)
; Output:
;   FileNotFoundException Could not locate test/metaprob/trace_test__init.class
;   or test/metaprob/trace_test.clj on classpath. Please check that namespaces
;   with dashes use underscores in the Clojure file name.  clojure.lang.RT.load
;   (RT.java:463)

It looks like Clojure is looking for only .clj files, not .cljc for some reason. (I'm running Clojure 1.9.0 so AFAIK it should be looking for .cljc...). I tried changing the ns metaprob.trace-test declaration at the top of test/metaprob/trace_test.cljc to ns test.metaprob.trace-test to match the filepath, same error.

[Might be irrelevant, see here] Part 1

Tried to run the tests following the instructions here:

(require 'metaprob.examples.all)
(require '[clojure.test :refer :all])
(run-all-tests)

This resulted in the following printout:

Testing clojure.spec.gen.alpha

Testing clojure.core.matrix.protocols

[... more stuff, omitted for brevity ...]

Testing clojure.core.matrix

Testing metaprob.examples.inference-on-gaussian

Ran 0 tests containing 0 assertions.
0 failures, 0 errors.
{:test 0, :pass 0, :fail 0, :error 0, :type :summary}

Tried the instructions for importing a specific test:

(require 'metaprob.trace-test)
; Output:
;   FileNotFoundException Could not locate metaprob/trace_test__init.class or
;   metaprob/trace_test.clj on classpath. Please check that namespaces with
;   dashes use underscores in the Clojure file name.  clojure.lang.RT.load
;   (RT.java:463)

It looks like these files used to be in the top-level metaprob directory, and at one point got moved to test/metaprob, but some needed path spec did not get updated.

bzinberg commented 5 years ago

Looking into it. I think I may have been mistaken for Part 1, there's no path problem because test/ is in the project's classpath. If that's correct, then Part 2 is the only cause for this issue.

bzinberg commented 5 years ago

The behavior I'm seeing seems to contradict the Clojure docs:

'require loads a lib by loading its root resource. The root resource path is derived from the lib name in the following manner: Consider a lib named by the symbol 'x.y.z; it has the root directory <classpath>/x/y/, and its root resource is <classpath>/x/y/z.clj, or <classpath>/x/y/z.cljc if <classpath>/x/y/z.clj does not exist.

bzinberg commented 5 years ago

Ah, I think I'm onto something. The test/ directory is in my classpath as reported by lein classpath:

lein classpath | tr ':' '\n'

# If there are a lot of uncached dependencies this might take a while ...
# /home/bzinberg/probcomp/metaprob/test
# /home/bzinberg/probcomp/metaprob/tutorial/src
# /home/bzinberg/probcomp/metaprob/src
# /home/bzinberg/probcomp/metaprob/dev-resources
# /home/bzinberg/probcomp/metaprob/tutorial/resources
# /home/bzinberg/probcomp/metaprob/target/classes
# /home/bzinberg/.m2/repository/clj-time/clj-time/0.11.0/clj-time-0.11.0.jar
# /home/bzinberg/.m2/repository/org/apache/httpcomponents/httpcore/4.1.2/httpcore-4.1.2.jar
# [... the rest of the paths are in `.m2` ...]

but from the Clojure REPL I get a different classpath that doesn't include test/:

(pprint (map #(.getFile %) (seq (.getURLs (java.lang.ClassLoader/getSystemClassLoader)))))

; ("/home/bzinberg/probcomp/metaprob/src/"
;  "/home/bzinberg/.m2/repository/org/clojure/clojure/1.9.0/clojure-1.9.0.jar"
;  "/home/bzinberg/.m2/repository/org/clojure/core.specs.alpha/0.1.24/core.specs.alpha-0.1.24.jar"
;  [... the rest of the paths are in `.m2` ...]
;  )
; nil
bzinberg commented 5 years ago

Running the REPL via lein repl instead of clj fixes the classpath (now it includes test/). If this is the right thing to do, it should be added to docs/interaction.md which currently says to use clj. Let me check if this fixes everything.

bzinberg commented 5 years ago

...yep.

bzinberg commented 5 years ago

The usage example should also include a line that imports all the test targets -- as currently written, (run-all-tests) will run 0 tests, since it only runs test targets that are already loaded. Drafted a fix for this in #151.

zane commented 5 years ago

I left some comments about this on #151.

zane commented 5 years ago

I agree that the instructions for running the test suite from the REPL are inaccurate as written. I left corrected instructions on #151.

We could amend interaction.md accordingly, but my preferred solution to this problem would be to just remove it entirely. The information contained therein is better captured by other documents from more authoritative sources [1][2].

bzinberg commented 5 years ago

Sure, that sounds like a good idea. I do think we should at least have some instructions on how to run the tests, even if it's as short as "Our tests follow Clojure conventions and can be run from the command line via the instructions here." Wdyt?

zane commented 5 years ago

Yeah, something along those lines sounds great. To be more precise: We should probably document what's unique about our test setup (our choice of aliases, our ClojureScript test runner, etc) and provide references to the official Clojure guides for everything else.

bzinberg commented 5 years ago

Great. I would volunteer to do this but I don't think I have the necessary knowledge. Would you or @alex-lew be up for taking this on?

zane commented 5 years ago

I can take it. Feel free to either make a new issue or reassign this one to me. 🙂

bzinberg commented 5 years ago

Thanks!

zane commented 5 years ago

Thank you for helping us make the documentation better!