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

Fix spec_dir lookup when Dir.pwd is not Rails.root #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kurko
Copy link

@kurko kurko commented Feb 4, 2018

Context

I was able to get this gem to work with my engine by going
inside the test_app dir and running

bundle exec rails spec:javascript

So far so good. The problem is that I don't want to have to run
cd spec/test_app every time to run my Javascript tests. I want to run them
from the engine's grandparent directory so I can include it in my Rakefile for
the entire test suite. In other words, this is my dir tree:

~/engine/
~/engine/spec/javascripts/
~/engine/spec/test_app/spec/javascripts/support/jasmine.yml

Rails.root is always ~/engine/spec/test_app/, regardless of whether
I'm in `~/engine/ or not.

The problem

Look at this line:

[paths].flatten.map { |path| Dir.glob path }.flatten.collect { |path| Rails.root.join(path) }

Dir.glob path, equivalent to Dir.glob("spec/javascripts") returns
true because Dir.pwd is ~/engine/, therefore
~/engine/spec/javascripts exists.

However, the last collect has Rails.root.join("spec_javascript),
which will map to ~/engine/spec/test_app/spec/javascripts, but
there's nothing there (because my test files were configured to
~/engine/spec/javascript).

To "fix that, I put ../javascripts in my yml's spec_dir, but no Dir.glob
just returns false because, in fact, ~/engine/../javascripts doesn't exist.
So, the code is verifying one path and resolving to some other.

The fix

-      [paths].flatten.map { |path| Dir.glob path }.flatten.collect { |path| Rails.root.join(path) }
+      [paths].flatten.map { |path| Dir.glob(Rails.root.join(path)) }.flatten.collect { |path| Rails.root.join(path) }

This fix just makes it cohesive.

I suspect this is in a way connected with
#102 (to make it work with
engines).

**Context**

I was able to get this gem to work with my engine by going
inside the `test_app` dir and running

`bundle exec rails spec:javascript`

So far so good. The problem is that I don't want to have to run
`cd spec/test_app` every time to run my Javascript tests. I want to run them
from the engine's grandparent directory so I can include it in my Rakefile for
the entire test suite. In other words, this is my dir tree:

```
~/engine/
~/engine/spec/javascripts/
~/engine/spec/test_app/spec/javascripts/support/jasmine.yml
```

`Rails.root` is always `~/engine/spec/test_app/`, regardless of whether
I'm in `~/engine/ or not.

**The problem**

Look at this line:

```rb
[paths].flatten.map { |path| Dir.glob path }.flatten.collect { |path| Rails.root.join(path) }
```

`Dir.glob path`, equivalent to `Dir.glob("spec/javascripts")` returns
`true` because `Dir.pwd` is `~/engine/`, therefore
`~/engine/spec/javascripts` exists.

However, the last `collect` has `Rails.root.join("spec_javascript)`,
which will map to `~/engine/spec/test_app/spec/javascripts`, but
there's nothing there (because my test files were configured to
`~/engine/spec/javascript`).

To "fix that, I put `../javascripts` in my yml's `spec_dir`, but no `Dir.glob`
just returns false because, in fact, `~/engine/../javascripts` doesn't exist.
So, the code is verifying one path and resolving to some other.

**The fix**

```patch
-      [paths].flatten.map { |path| Dir.glob path }.flatten.collect { |path| Rails.root.join(path) }
+      [paths].flatten.map { |path| Dir.glob(Rails.root.join(path)) }.flatten.collect { |path| Rails.root.join(path) }
```

This fix just makes it cohesive.

I suspect this is in a way connected with
testdouble#102 (to make it work with
engines).
@kurko
Copy link
Author

kurko commented Feb 4, 2018

erm, don't think the failure is related:

<--- Running rake spec:javascript
rake aborted!
NoMethodError: private method `include' called for ActionDispatch::Assertions:Module
/home/travis/.rvm/gems/ruby-2.0.0-p648/gems/turbolinks-5.1.0/lib/turbolinks.rb:25:in `block (2 levels) in <class:Engine>'

@@ -23,7 +23,7 @@ def route_path

def spec_dir
paths = jasmine_config['spec_dir'] || 'spec/javascripts'
[paths].flatten.map { |path| Dir.glob path }.flatten.collect { |path| Rails.root.join(path) }
[paths].flatten.map { |path| Dir.glob(Rails.root.join(path)) }.flatten.collect { |path| Rails.root.join(path) }
Copy link

@kyrofa kyrofa Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[paths].flatten.map { |path| Dir.glob(Rails.root.join(path)) }.flatten.collect { |path| Rails.root.join(path) }
[paths].flatten.collect { |path| Pathname.glob Rails.root.join(path) }.flatten

That's how I fixed it, anyway. Seems a little more efficient and cleaner. And actually, I think those globs are going to include the rails root, which means trying to join it again will lead to issues, no?

Anyway, I was going to propose it, but then I saw you already tried to fix it years ago. @searls what do you think about this? I ended up proposing #240 with this just given the age.

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.

2 participants