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

Make JRuby before command hook work on Aruba environment #610

Merged
merged 1 commit into from
May 4, 2019

Conversation

mvz
Copy link
Contributor

@mvz mvz commented May 3, 2019

Summary

Make the JRuby before command helper modify the Aruba environment, rather than the real one.

Details

The JRuby helper sets up a before :command hook to set helpful environment variables for JRuby. This changes makes the helper use set_environment_variable to set Aruba's environment rather than directly modifying the real one.

Motivation and Context

When the environment is modified directly, this causes unexpected behavior when using with_environment, since that methods will base the changed environment on the environment at startup, rather than the current one. Therefore, with_environment will unexpectedly undo such modifiections.

All this came to light while working on #603.

How Has This Been Tested?

Tests for the JRuby helper were updated to check the result inside a with_environment block, thus mimicing the behavior when the hook is used before running a command.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I've added tests for my code

Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests now read exceptionally well, and the implementation code is clearer as well.

And the tests pass!

@mvz mvz merged commit 24c0675 into still May 4, 2019
@mvz mvz deleted the improve-jruby-env branch May 4, 2019 07:09
@mvz mvz mentioned this pull request May 5, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants