Skip to content

Fixing issue where bundler would get confused during multi-app selenium#3

Merged
dtognazzini merged 1 commit intomasterfrom
bundler_gemfile_fix
Dec 15, 2012
Merged

Fixing issue where bundler would get confused during multi-app selenium#3
dtognazzini merged 1 commit intomasterfrom
bundler_gemfile_fix

Conversation

@amutz
Copy link
Copy Markdown
Contributor

@amutz amutz commented Dec 15, 2012

running just "rake" from the gem root directory should now do what it is supposed to.

@dtognazzini
Copy link
Copy Markdown
Contributor

I verified all of the following work:
rake
bundle exec rake
bundle exec rake test:units
rake test:integration:units
bundle exec rake test:integration:units
rake test:integration:selenium
bundle exec rake test:integration:selenium
APP_VERSION='3.x' rake test:integration:selenium
APP_VERSION='3.x' bundle exec rake test:integration:selenium

This does not work:
rake test:units

...which is probably fine since we're using bundler, so we need to 'bundle
exec' for tasks that require it.

All of this still kind of smells to me:

  1. test:units requires bundle exec since it needs to use the Gemfile at the
    gem root.
  2. test:integration:units doesn't need to use bundle exec 'cause we're
    depending on Appraisal to do that for us. Additionally, they use a "bundler
    free"environment in the event that the task is invoked using bundle exec.
    Yet, they also "leak" the bundler environment (see below).
  3. test:integration:selenium doesn't want to use bundle exec 'cause we're
    changing directories and running the task within the context of a Rails
    application. Additionally, we need to add code to ensure a "bundler free"
    environment in the event that the task is invoked using bundle exec.
  4. The default rake task combines test:integration:units and
    test:integration:selenium, which means each task really needs to protect
    itself from the possibility of being invoked using bundle exec.

The last point is perhaps the most interesting as this is what caused
'rake' to fail. There are certain tasks that require a bundler environment
and others that require a non-bundler environment as they themselves choose
a bundler environment (either explicitly like Appraisal, or implicitly like
test:integration:selenium).

The reason why the default rake task failed is because
test:integration:units as implemented by Appraisal "leaks" the bundler
environment by leaving the BUNDLE_GEMFILE env variable assigned; when
test:integration:selenium is invoked, the BUNDLE_GEMFILE env variable is
used instead of the implicit resolution which would look for the Gemfile in
the Rails application root.

Perhaps the guideline to follow is that for any task that is going to run
tasks on a Gemfile that it selects, it should always use a "bundler free"
environment to protect itself from 1) someone invoking with bundle exec and
2) being included in the same process space as another task that leaks the
bundler environment.

So, if that's the guideline, then we should have a standard way of doing
that.

On Fri, Dec 14, 2012 at 5:23 PM, Andrew Mutz notifications@github.comwrote:

running just "rake" from the gem root directory should now do what it is

supposed to.

You can merge this Pull Request by running:

git pull https://github.com/appfolio/ae_page_objects bundler_gemfile_fix

Or view, comment on, or merge it at:

#3
Commit Summary

  • Fixing issue where bundler would get confused during multi-app
    selenium

File Changes

  • M Rakefile (10)

Patch Links

Donnie Tognazzini

Principal Software Engineer | AppFolio, Inc.
P: 805.617.2167 | F: 805.968.0646

donnie.tognazzini@appfolio.com

Find Us Online --
www.AppFolio.com
www.Facebook.com/AppFolio
www.PropertyManager.com
www.GreenPropertyManagement.com

dtognazzini added a commit that referenced this pull request Dec 15, 2012
Fixing issue where bundler would get confused during multi-app selenium
@dtognazzini dtognazzini merged commit a736d4d into master Dec 15, 2012
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.

2 participants