tests on master should have no failures on any computer

[tute]

Right now they are only green in CI. Different computers fail differently, in mine I get:

rspec ./spec/paperclip/validators_spec.rb:21 # Paperclip::Validators using the helper prevents you from attaching a file that violates that validation
rspec ./spec/paperclip/storage/s3_spec.rb:696 # Paperclip::Storage::S3 Parsing S3 credentials with a s3_host_name in them gets the right s3_host_name if the key does not exist
rspec ./spec/paperclip/attachment_processing_spec.rb:52 # Attachment Processing using validates_attachment processes attachments given a valid assignment

[tute]

rspec ./spec/paperclip/validators_spec.rb:21 seem to depend on the seed, the others two fail "more reliably".

[pedrosmmoreira]

Hi @tute, happy to help out with this one if possible. Running the specs now

[tute]

Thank you! 😃

[pedrosmmoreira]

Thank you and all the maintainers for your time and effort :)

[pedrosmmoreira]

rspec ./spec/paperclip/storage/s3_spec.rb:696 is failing to create a new Aws::Resource on https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip/storage/s3.rb#L247 as the test setup neither sets a test key nor an s3_region key. Since the Aws sdk does regex match on the passed in string, we fail on NoMethodError

I don't know how realistic this testing scenario: would not a failure to include the s3 hash in options passed to has_attached_file cause a fallback to whatever is defined in the environment? In that case, nothing being found, Paperclip would fallback to setting storage to :filesystem, correct?

Seems like we have a quick fix by just adding the test env hash with s3_region pointing to an empty string (in https://github.com/thoughtbot/paperclip/blob/master/spec/paperclip/storage/s3_spec.rb#L667).

I can PR this if you agree that's a reasonable solution.

[tute]

That is a very weird spec. Why do we need the rails_env("test") block?

Seems like we have a quick fix by just adding the test env hash with s3_region pointing to an empty string (in https://github.com/thoughtbot/paperclip/blob/master/spec/paperclip/storage/s3_spec.rb#L667).

I can PR this if you agree that's a reasonable solution.

That makes sense to me. 👍 Thanks for your help!

[pedrosmmoreira]

I'll push that PR up in a second. I'll have a look at another PR to remove the block and continue investigating failures.

[tute]

I'll have a look at another PR to remove the block and continue investigating failures.

Another question: why doesn't it fail in CI?

[tute]

Ohh. Those tests don't run in CI, as they need ENV vars, right? Which is another weird thing of paperclip's test suite, I think.

[pedrosmmoreira]

Yeah, the situation is slightly strange, but with some help we can push to get this to a good place!

[pedrosmmoreira]

rspec ./spec/paperclip/attachment_processing_spec.rb:52 seems to be failing due the rebuild_class before_hook being restricted to only one of the contexts (https://github.com/thoughtbot/paperclip/blob/master/spec/paperclip/attachment_processing_spec.rb#L6).
This is bound to leave the Dummy class in a bad state and cause further issues downstream. Prepping a PR for this one now.

[pedrosmmoreira]

rspec ./spec/paperclip/validators_spec.rb:21 seems once again a case of incorrect setup. Adding the call to rebuild_class to https://github.com/thoughtbot/paperclip/blob/master/spec/paperclip/validators_spec.rb#L5 for it to reliably pass.

[pedrosmmoreira]

@tute let me know if you prefer these PRs to be cherry-picked into one.

[tute]

Thank you, @pedrosmmoreira!