Tests fail if directory has a period (.) in the path

Description

Check out compiler into a cljs.dev directory and run lein test :only cljs.module-processing-tests.

Seems very similar to CLJS-2914.

Environment

None

Activity

Show:
Mike Fikes
September 26, 2018, 3:06 AM

Hey Ray, this patch doesn't apply on master. Is the intent that this patch depend on the patch in being applied first? (Usually patches are independent, but in some cases a patch can depend on changes in another ticket if it needs to.)

Ray Mcdermott
September 26, 2018, 10:25 AM

Hi Mike - yes the patch for needs to be applied first. If it was done the other way around, that patch would revert this one.

Another option would be to close the other bug with a new patch here that squashes both bugs.

I was concerned about trying to close two bugs with one patch but yes the ordering / dependency is not good either.

How would you prefer to handle it?

Mike Fikes
September 26, 2018, 12:30 PM

Since one patch revises a line and another patch adds a new line, you could normally make independent patches where either could be applied first. But, the problem with this pair of tickets, though, is that the changes are sufficiently close together that whichever patch is applied second will need to be re-baselined. (I don't know what the threshold is, but anywhere within a few lines will do it.) To avoid that, we might as well have this patch depend on the one in being applied first.

Mike Fikes
September 26, 2018, 1:08 PM

CLJS-2915.patch LGTM. Note that it is designed to be applied after the patch in CLJS-2782.

Tested locally with a checkout in a directory with both dots and hyphens. Also passes in CI.

David Nolen
September 28, 2018, 5:50 PM
Completed

Assignee

David Nolen

Reporter

Ray Mcdermott

Labels

None

Approval

None

Patch

Code

Affects versions

Priority

Major
Configure