Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't return incorrect files when there's a file whose name matches a dir #526

Merged
merged 3 commits into from Dec 31, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/listen/record.rb
Expand Up @@ -48,7 +48,8 @@ def dir_entries(rel_path)
subtree = if [nil, '', '.'].include? rel_path.to_s
@tree
else
_sub_tree(rel_path)
@tree[rel_path.to_s] ||= _auto_hash
@tree[rel_path.to_s]
end

subtree.transform_values do |values|
Expand Down
23 changes: 23 additions & 0 deletions spec/acceptance/listen_spec.rb
Expand Up @@ -208,6 +208,29 @@
end
end

context 'listens to sub-subdirectory removed' do
around do |example|
mkdir_p 'dir1'
mkdir_p 'dir1/subdir1'
mkdir_p 'dir1/subdir1/subdir2'
touch 'dir1/subdir1/file.rb'
touch 'dir1/subdir1/subdir2/file.rb'
example.run
end

it 'listen to the files of a removed directory' do
expected = {
modified: [],
added: [],
removed: %w[dir1/subdir1/file.rb dir1/subdir1/subdir2/file.rb]
}

expect(wrapper.listen do
rm_rf 'dir1/subdir1'
end).to eq expected
end
end

context 'with .bundle dir ignored by default' do
around do |example|
mkdir_p '.bundle'
Expand Down
35 changes: 33 additions & 2 deletions spec/lib/listen/record_spec.rb
Expand Up @@ -208,6 +208,29 @@ def record_tree(record)
end
end

context 'when there is a file with the same name as a dir' do
subject { record.dir_entries('cypress') }

before do
record.update_file('cypress.json', mtime: 1.1)
record.update_file('cypress/README.md', mtime: 1.2)
record.update_file('a/b/cypress/d', mtime: 1.3)
record.update_file('a/b/c/cypress', mtime: 1.3)
end
it { should eq('README.md' => { mtime: 1.2 }) }
end

context 'when there is a file with a similar name to a dir' do
subject { record.dir_entries('app') }

before do
record.update_file('appspec.yml', mtime: 1.1)
record.update_file('app/README.md', mtime: 1.2)
record.update_file('spec/app/foo', mtime: 1.3)
end
it { should eq('README.md' => { mtime: 1.2 }) }
end

context 'in subdir /path' do
subject { record.dir_entries('path') }

Expand All @@ -220,9 +243,17 @@ def record_tree(record)
it { should eq('file.rb' => { mtime: 1.1 }) }
end

context 'with path/subdir' do
context 'with empty path/subdir' do
before { record.add_dir('path/subdir') }
it { should eq('subdir' => {}) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bryanlira you added this test in #460. Do you remember why you expected it to return this, vs an empty hash? The acceptance test you added in that PR is still passing with this branch, so I'm not sure if this unit test was critical for something else that I'm not seeing it.

it { should be_empty }
end

context 'with path/subdir with file' do
before do
record.add_dir('path/subdir')
record.update_file('path/subdir/file.rb', mtime: 1.1)
end
it { should be_empty }
end
end
end
Expand Down