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 delete /tmp before generator tests #1744

Merged
merged 1 commit into from
Aug 21, 2020

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Aug 20, 2020

I think I finally discovered why the directory /tmp was being deleted, a behaviour that has been confusing us lately. (See #1731, #1726, #1695).

The issue appears to be with the generator specs. These load this configuration when they run:

RSpec.configure do |config|
config.include GeneratorSpecHelpers
config.before(:example, :generator) do
destination File.expand_path("../../../tmp", __FILE__)
prepare_destination
end
end

In this code, destination and prepare_destination are helpers provided by Rails to assist when testing generators. See https://api.rubyonrails.org/v3.2.2/classes/Rails/Generators/TestCase.html

Turns out that prepare_destination prepares the destination by... rimraffing it. See: https://github.com/rails/rails/blob/557014d45aeed82ed1e79b83ab59697392a1a5de/railties/lib/rails/generators/test_case.rb#L235

So if we set /tmp as destination, it will be destroyed ahead of each generator spec example.

To fix this, I'm telling it to use a specific subdirectory that can be deleted at will, avoiding further harm.

@pablobm pablobm force-pushed the tmp-deletion-culprit branch from 175d07b to 9a4f0c9 Compare August 20, 2020 17:16
Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

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

Oh! That explains a lot.

@nickcharlton nickcharlton merged commit 9eb7e5f into thoughtbot:master Aug 21, 2020
@pablobm pablobm deleted the tmp-deletion-culprit branch June 10, 2021 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants