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

Fix: FileStoreTest#test_filename_max_size fails in Ruby 2.5.1 #32386

Closed
wants to merge 1 commit into from

Conversation

utilum
Copy link
Contributor

@utilum utilum commented Mar 30, 2018

Summary

As of Tmpname.create was changed in ruby/ruby@10b9690 to return a 'path' purged of file separators. The result is a test failure, even though the actual basename respects the constraint:

FileStoreTest#test_filename_max_size [/rails/activesupport/test/cache/stores/file_store_test.rb:72]:
Temp filename too long: 297

Other Information

$ ruby -v
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]

@rails-bot
Copy link

r? @kaspth

(@rails-bot has picked a reviewer for you, use r? to override)

@pixeltrix
Copy link
Contributor

@utilum we can fix this in a better way by using the second argument to Dir::Tmpname.create, e.g:

def test_filename_max_size
  key = "#{'A' * ActiveSupport::Cache::FileStore::FILENAME_MAX_SIZE}"
  path = @cache.send(:normalize_key, key, {})
  basename = File.basename(path)
  dirname = File.dirname(path)

  Dir::Tmpname.create(basename, Dir.tmpdir + dirname) do |tmpname, n, opts|
    assert File.basename(tmpname + ".lock").length <= 255, "Temp filename too long: #{File.basename(tmpname + '.lock').length}"
  end
end

That way we're not changing the asserted value

@pixeltrix pixeltrix assigned pixeltrix and unassigned kaspth Mar 31, 2018
@pixeltrix
Copy link
Contributor

@utilum don't worry - I manually edited your commit and merged it in 0548d65. Thanks for spotting the error! 👍

@pixeltrix pixeltrix closed this Mar 31, 2018
pixeltrix added a commit that referenced this pull request Mar 31, 2018
Fix: FileStoreTest#test_filename_max_size fails in Ruby 2.5.1
@utilum utilum deleted the tmpname_basename branch March 31, 2018 13:51
pixeltrix added a commit that referenced this pull request Mar 31, 2018
The removal of path separators from the tmpname value affects all four
supported versions of Ruby - 2.2.10, 2.3.7, 2.4.4, and 2.5.1.
pixeltrix added a commit that referenced this pull request Apr 2, 2018
The removal of path separators from the tmpname value affects all four
supported versions of Ruby - 2.2.10, 2.3.7, 2.4.4, and 2.5.1.
pixeltrix added a commit that referenced this pull request Apr 2, 2018
The removal of path separators from the tmpname value affects all four
supported versions of Ruby - 2.2.10, 2.3.7, 2.4.4, and 2.5.1.
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.

4 participants