ClojureScript

lein test fails if directory has hyphens

Details

  • Patch:
    Code

Description

Create a directory with hyphens in its name, and then clone clojurescript into it and run lein test.

lein test :only cljs.module-processing-tests/commonjs-module-processing-preprocess-symbol

FAIL in (commonjs-module-processing-preprocess-symbol) (module_processing_tests.clj:191)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/es6-module-processing

FAIL in (es6-module-processing) (module_processing_tests.clj:97)
Processed modules are added to :js-module-index
expected: (= {"es6-hello" {:name (absolute-module-path "src/test/cljs/es6_hello.js"), :module-type :es6}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"es6-hello" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$es6-hello", :module-type :es6}} {"es6-hello" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$es6-hello", :module-type :es6}}))

lein test :only cljs.module-processing-tests/test-cljs-1822

FAIL in (test-cljs-1822) (module_processing_tests.clj:162)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/react-min.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle-min.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/commonjs-module-processing

FAIL in (commonjs-module-processing) (module_processing_tests.clj:71)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

Perhaps related to CLJS-2703.

Activity

Hide
Ray McDermott added a comment -

Problem is that when munge uses the CLJ option, it emits the folder with `_` rather than `-`.

This constraint doesn't always seems to be applicable for CLJS.

Show
Ray McDermott added a comment - Problem is that when munge uses the CLJ option, it emits the folder with `_` rather than `-`. This constraint doesn't always seems to be applicable for CLJS.
Hide
Ray McDermott added a comment -

Note that the fix for CLJS-2703 does not enable the tests to pass

Show
Ray McDermott added a comment - Note that the fix for CLJS-2703 does not enable the tests to pass
Hide
Ray McDermott added a comment -

One option could be to exclude the directory path from munging.

munge is already quite complex so having an additional special case might be undesirable.

Show
Ray McDermott added a comment - One option could be to exclude the directory path from munging. munge is already quite complex so having an additional special case might be undesirable.
Hide
Ray McDermott added a comment -

Another option is to have the tests support both munged and unmanaged paths locally.

It's also a little confusing but keeps the changes in the test code which is probably preferred.

Show
Ray McDermott added a comment - Another option is to have the tests support both munged and unmanaged paths locally. It's also a little confusing but keeps the changes in the test code which is probably preferred.
Hide
Ray McDermott added a comment -

A mix of using the real / patched base directory works.

Seems like a hack solution but I'll create a patch for review.

Show
Ray McDermott added a comment - A mix of using the real / patched base directory works. Seems like a hack solution but I'll create a patch for review.
Hide
Mike Fikes added a comment -

Hey Ray, you're right in that the change for CLJS-2703 broke this. That patch ended up fixing things for test-module-name-substitution by introducing a (string/replace $ "-" "_").

It turns out that test-module-name-substitution has the only places where the code? parameter in absolute-module-path is passed as true, so it seems that this could be leveraged to fix things. In short, the (string/replace $ "-" "_") introduced with CLJS-2703 could be changed to only conditionally do it by changing that offending line to instead be

(cond-> $ code? (string/replace "-" "_"))
Show
Mike Fikes added a comment - Hey Ray, you're right in that the change for CLJS-2703 broke this. That patch ended up fixing things for test-module-name-substitution by introducing a (string/replace $ "-" "_"). It turns out that test-module-name-substitution has the only places where the code? parameter in absolute-module-path is passed as true, so it seems that this could be leveraged to fix things. In short, the (string/replace $ "-" "_") introduced with CLJS-2703 could be changed to only conditionally do it by changing that offending line to instead be
(cond-> $ code? (string/replace "-" "_"))
Hide
Ray McDermott added a comment -

Ha - yeah, that's great and I've just tested it and works like a charm. I make a new patch.

Show
Ray McDermott added a comment - Ha - yeah, that's great and I've just tested it and works like a charm. I make a new patch.
Hide
Ray McDermott added a comment -

cond-> fix

Show
Ray McDermott added a comment - cond-> fix
Hide
Ray McDermott added a comment -

We can probably use the same fix approach for CLJS-2915.

I look forward to the emoji and unicode reports.

Show
Ray McDermott added a comment - We can probably use the same fix approach for CLJS-2915. I look forward to the emoji and unicode reports.
Hide
Mike Fikes added a comment -

CLJS-2782.patch LGTM. It is safe to apply as it only revises the test code like CLJS-2703. (As was deemed with CLJS-2703, this is probably only a problem with the test code and there is no real issue with the production code).

I tried it with a directory with hyphens and it worked for me, and it also passes in CI.

Show
Mike Fikes added a comment - CLJS-2782.patch LGTM. It is safe to apply as it only revises the test code like CLJS-2703. (As was deemed with CLJS-2703, this is probably only a problem with the test code and there is no real issue with the production code). I tried it with a directory with hyphens and it worked for me, and it also passes in CI.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: