thoughtbot / appraisal

A Ruby library for testing your library against different versions of dependencies.
https://thoughtbot.com
MIT License
1.25k stars 107 forks source link

Use bundler original env when running bundle commands. Fixes #173 #174

Closed excid3 closed 1 year ago

excid3 commented 3 years ago

This PR fixes the situation when you've got something like a path config set in bundler.

± bundle config path vendor/bundle

± bundle exec appraisal
true
>> bundle check --gemfile='/Users/chris/code/acts_as_tenant/gemfiles/rails_5.gemfile' || bundle install --gemfile='/Users/chris/code/acts_as_tenant/gemfiles/rails_5.gemfile' --retry 1
Traceback (most recent call last):
    1: from /Users/chris/.asdf/installs/ruby/2.7.2/bin/bundle:23:in `<main>'
/Users/chris/.asdf/installs/ruby/2.7.2/bin/bundle:23:in `load': cannot load such file -- /Users/chris/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle (LoadError)
Traceback (most recent call last):
    1: from /Users/chris/.asdf/installs/ruby/2.7.2/bin/bundle:23:in `<main>'
/Users/chris/.asdf/installs/ruby/2.7.2/bin/bundle:23:in `load': cannot load such file -- /Users/chris/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle (LoadError)

Removing RUBYOPT when resetting the ENV fixes the above, however we can use the built-in Bundler helper instead to properly clean it for sub-commands.

Fixes #173

excid3 commented 3 years ago

Since this modifies the ENV, I wasn't sure how best to fix the test suite. Would love pointers!

julianrubisch commented 3 years ago

This PR fixed a GH action for me that raised

ArgumentError: Trying to register Bundler::GemfileError for status code 4

nickcharlton commented 3 years ago

Thanks for opening this @excid3!

I tried it out on a project a while ago, then got completely swamped elsewhere, so I'm only just coming back to this.

Basically, I think this looks good, but we need to do something about the test failures. I had a look today but I'm not really sure where to start either. My immediate feeling was that the suite might be doing something in the acceptable test helpers: spec/support/acceptance_test_helpers.rb. Did you have any thoughts originally or since about what we might be able to do here?

pboling commented 2 years ago

Jumping in this thread to subscribe and remind myself to try to spend time fixing this. The alternatives are:

  1. Rip Appraisal out from many gems I am starting to switch to Github Actions
    • lose the functionality it provides
    • support a smaller range of compatibility in my libraries
  2. Fix Appraisal to work with latest bundler and GH Actions 🥳

You seem to be the lone maintainer of Appraisal @nickcharlton - thanks for your efforts! It has been a very useful tool!

Things I need, and hope to work on in the coming weeks:

nickcharlton commented 2 years ago

Hi @pboling, thanks for commenting above and sorry it's been a long time!

I'd be very excited to get some help with this project; this PR is the main thing holding us from doing a release and fixing a lot of problems for people and I've not been able to make time to work on it in recent months. Is this something you've been able to look at?

pboling commented 2 years ago

@nickcharlton I haven't yet. I have barely had time to touch my open source since I commented. :(. I'm getting settled more into a routine now though. Perhaps soon I will be able to have a look.

pboling commented 2 years ago

Of note - Github's view_component gem recently set up Appraisals with Github Actions and it nearly worked... not quite 💯...

https://github.com/github/view_component/pull/1225

nickcharlton commented 2 years ago

@pboling, I know that feeling! …unfortunately.

The main thing stopping us right now is this issue. This particular fix is okay, but without tests, I'm not willing to merge it in.

Towards the end of last year, I was working on running Appraisal's tests without faking out the environment, which, with recent bundler changes seemed the best next step. I didn't make enough progress to contribute to this PR, and I don't think I'm going to be able to find the time to do so either. But maybe that gives you (or anyone else!) a possible next step?

excid3 commented 2 years ago

I haven't had any time to jump back into this either. It did break the tests, so they need updating.

I wasn't sure if it needed any additional tests, and if so, what should those test?

For now, I have Appraisal generate the gemfiles and GitHub Actions set the BUNDLE_GEMFILE for each run.

nickcharlton commented 2 years ago

I don't think we need anything additional, just for the tests themselves to now pass.

I had real trouble trying to adjust the tests though — looking back through my WIP, I'd given up trying to replicate the environment bundler expects as I couldn't get it to run in a temporary environment and still actually work.

Spone commented 2 years ago

Hi @excid3 can you rebase your branch from thoughtbot:main? There have been new versions since you opened the PR.

Of note - Github's view_component gem recently setup Appraisals with Github Actions and it all worked

It worked, until we tried running the Appraisal commands in Github Actions. I'm currently working on it (https://github.com/github/view_component/pull/1230/). It works when using this branch (when rebased from main, else it fails on Ruby 3+).

kyrofa commented 1 year ago

Tests fixed in #202.

nickcharlton commented 1 year ago

I've now merged in #202.

Thank you everyone for your patience with this issue, especially everyone who contributed along the way and kept it moving along. As I've written elsewhere, this has been a big headache for the project, so I'm sure this will now unblock a lot of improvements that we'd all like to see.