-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Use bundler original env when running bundle commands. Fixes #173 #174
Conversation
Since this modifies the ENV, I wasn't sure how best to fix the test suite. Would love pointers! |
This PR fixed a GH action for me that raised
|
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: |
Jumping in this thread to subscribe and remind myself to try to spend time fixing this. The alternatives are:
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:
|
* DEV: Update the CI workflow * DEV: Update mini_racer * DEV: Don't use appraisal gem in CI (see: thoughtbot/appraisal#174) * DEV: Don't fail fast * FIX: A regression in #199
* DEV: Update the CI workflow * DEV: Update mini_racer * DEV: Don't use appraisal gem in CI (see: thoughtbot/appraisal#174) * DEV: Don't fail fast * FIX: A regression in discourse#199
* DEV: Update the CI workflow * DEV: Update mini_racer * DEV: Don't use appraisal gem in CI (see: thoughtbot/appraisal#174) * DEV: Don't fail fast * FIX: A regression in discourse#199
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? |
@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. |
Of note - Github's |
@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? |
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. |
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. |
Hi @excid3 can you rebase your branch from
It worked, until we tried running the Appraisal commands in Github Actions. I'm currently working on it (ViewComponent/view_component#1230). It works when using this branch (when rebased from main, else it fails on Ruby 3+). |
8bb003b
to
14855fc
Compare
Tests fixed in #202. |
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. |
This PR fixes the situation when you've got something like a path config set in bundler.
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