RSpec should prefer SPEC_OPTS to cmdline options (regression in 2.4.0) #276

Closed
iromeo opened this Issue Jan 13, 2011 · 8 comments

Comments

Projects
None yet
2 participants
@iromeo

iromeo commented Jan 13, 2011

Hello,

After "Restore --options CLI option from rspec-1" (d7e4033) refactoring RSpec 2.0 starts prefer cmdline options to options preferred via env variables. According to diff the bug is evident:

[global_options, local_options, command_line_options, env_options].inject do |merged, options|

was replaced to
....
options_to_merge << env_options
options_to_merge << command_line_options

Thus merge order was occasionally changed. Fix is just to swap this two lines.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Jan 13, 2011

Member

Do you want to submit a patch? If not I'll take care of it.

Member

dchelimsky commented Jan 13, 2011

Do you want to submit a patch? If not I'll take care of it.

@iromeo

This comment has been minimized.

Show comment
Hide comment
@iromeo

iromeo Jan 13, 2011

At the moment I didn't managed to correctly configure rspec-dev so I cannot attach a test for my fix.

iromeo commented Jan 13, 2011

At the moment I didn't managed to correctly configure rspec-dev so I cannot attach a test for my fix.

@iromeo

This comment has been minimized.

Show comment
Hide comment
@iromeo

iromeo Jan 13, 2011

David, I've configured rspec-dev environment, I'll attach a patch

iromeo commented Jan 13, 2011

David, I've configured rspec-dev environment, I'll attach a patch

@iromeo

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Jan 17, 2011

Member

Sorry for not asking this before, but I didn't really think about it until going to merge this.

My instinct is that command line options should take precedence over anything else, even those defined in SPEC_OPTS. Is there a problem that is happening as a result of this ordering?

Member

dchelimsky commented Jan 17, 2011

Sorry for not asking this before, but I didn't really think about it until going to merge this.

My instinct is that command line options should take precedence over anything else, even those defined in SPEC_OPTS. Is there a problem that is happening as a result of this ordering?

@iromeo

This comment has been minimized.

Show comment
Hide comment
@iromeo

iromeo Jan 17, 2011

At least rspec rake task passes its options using cmdline thus RubyMine and our CI Server (TeamCity) cannot patch user's rake task options via SPEC_OPTS env variable. We don't force user to adapt his rake tasks for our IDE / CI. I suppose it is better to allow the same code to be executed in console and in our products. The difference is only in results presentation but implementation details are hidden.

My instinct is that command line options should take precedence over anything else

I suppose both ways are reasonable. SPEC_OPTS priority is better for patching default rspec behaviour.

iromeo commented Jan 17, 2011

At least rspec rake task passes its options using cmdline thus RubyMine and our CI Server (TeamCity) cannot patch user's rake task options via SPEC_OPTS env variable. We don't force user to adapt his rake tasks for our IDE / CI. I suppose it is better to allow the same code to be executed in console and in our products. The difference is only in results presentation but implementation details are hidden.

My instinct is that command line options should take precedence over anything else

I suppose both ways are reasonable. SPEC_OPTS priority is better for patching default rspec behaviour.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Jan 17, 2011

Member

Prefer SPEC_OPTS to cli

Member

dchelimsky commented Jan 17, 2011

Prefer SPEC_OPTS to cli

@iromeo

This comment has been minimized.

Show comment
Hide comment
@iromeo

iromeo Jan 18, 2011

Thanks!

iromeo commented Jan 18, 2011

Thanks!

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment