simonc / memfs

MemFs provides a fake file system that can be used for tests. Strongly inspired by FakeFS.
MIT License
320 stars 27 forks source link

Update Ruby versions on Travis CI #30

Closed aried3r closed 4 years ago

aried3r commented 4 years ago

Let's see if this just works :)

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.2%) to 99.779% when pulling 621b81bae4f14e6fdabae9e9453c05191aff4540 on aried3r:patch-1 into 26e0cb91643f98c523352106f135dc0171d86987 on simonc:master.

aried3r commented 4 years ago

Hmm, quite a long list of failures on Ruby 2.6. I'll try to have a look.

simonc commented 4 years ago

Hi @aried3r!

Thanks for opening this PR. I haven't been very active on the project for a while but I can help get it working for Ruby 2.6.

If you have any question or if you want to have a pairing session I'm available (French timezone though ^^).

Cheers !

aried3r commented 4 years ago

Ok, so some of the failures are indeed because of changes in Ruby 2.6.

Ruby 2.6 includes version 1.1 of the fileutils gem, which includes this change: https://github.com/ruby/fileutils/pull/13. fileutils is now a stdgem (https://stdgems.org/), but I think that's unrelated. In this case, I'd just adopt the expected result if RUBY_VERSION >= 2.6, WDYT?

The errors related to missing children are because of the addition of Dir.children, however I haven't looked into it how exactly this gem is using this, directly or indirectly, especially since it fails on MemFs::Dir. Maybe you can shed some light on how to best fix this?

https://github.com/ruby/fileutils/commit/d8c665c564e5d4e54da1c92788613f524aad0de2 https://github.com/ruby/fileutils/commit/bc92c930c7acd591ae99beafa6841700b4683bc1

simonc commented 4 years ago

@aried3r The condition on RUBY_VERSION seems to be a sensible choice. I don't see a more explicit solution for this issue.

Regarding the missing children method it's failing on MemFs::Dir because it's the class replacing the original Ruby Dir class when MemFs is enabled. I think the next step is to implement this method in MemFs::Dir.

The only interrogation I have is if we should condition this method's definition with if RUBY_VERSION >= 2.6 🤔

My pro argument is that it would prevent this method from being defined in earlier versions and thus prevent unwanted effects in users specs.

My con argument is that no other method was conditioned by the current Ruby version before so I'm not sure it'd make sense to do it in this specific occasion.

What's your opinion on that matter?

aried3r commented 4 years ago

Hey! Seems like I pushed some changes but never followed up here. So I added the necessary methods and also made them dependent on the Ruby version installed. WDYT?

aried3r commented 4 years ago

I knew I shouldn't have removed EOL Rubies and added Ruby 2.7 in one commit 😅

Now it gets interesting. In Ruby 2.7 File::extname returns . on non-Windows platforms. Do you want to handle this case (if Ruby >= 2.7 && !Windows (https://github.com/ruby/ruby/pull/2565/files#diff-01bda6262e55b3f376e1fd6ad669f78aR242))?

https://docs.ruby-lang.org/en/2.7.0/File.html#method-c-extname https://docs.ruby-lang.org/en/2.7.0/NEWS.html https://rubyreferences.github.io/rubychanges/2.7.html#fileextname-returns-a--string-at-a-name-ending-with-a-dot

aried3r commented 4 years ago

I added the Ruby 2.7 behavior of File::extname to the specs.

simonc commented 4 years ago

@aried3r Thanks for your work on this! ❤️