# Code Review: fixes-2

**URL:** <https://www.ruby-forum.com/t/code-review-fixes-2/142556>\
**Category:** IronRuby\
**Created:** [July 9, 2008, 6:37am UTC](https://www.ruby-forum.com/t/code-review-fixes-2/142556 "2008-07-09T06:37:28Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [July 9, 2008, 6:37am UTC](https://www.ruby-forum.com/t/code-review-fixes-2/142556/1 "2008-07-09T06:37:28Z")

</div>

Ruby only. All local tests pass.

tfpt review “/shelveset:fixes-2;REDMOND\jflam”

Comment :

- 

```
Added a ChildProcessExitStatus global variable ($?) which 

```

contains the exit code of a sub-process (used by system, backtick, exec  
etc)

- 

```
Added Glob.cs to Ruby/Ruby/Builtins. This contains Ruby glob 

```

functionality that previously resided in the libraries (FileOps.cs and  
Dir.cs), but was moved so that we can do glob expansions for globs  
passed in as command-line parameters to the Ruby script. We now expand  
parameters that look like glob expansions before adding them to ARGV.

- 

```
Changed RUBY_INSTALL_NAME to ironruby from ruby in rbconfig.rb

```

- 

```
Added a Process::Status class

```

- 

```
Moved File.CanonicalizePath to from FileOps.cs to Glob.cs since 

```

this will be used in multiple places in the future.

- 

```
Added a new algorithm for determining what the user's HOME path 

```

is. Ruby uses HOME for several library methods such as Dir.chdir(). The  
new algorithm tries HOME, HOMEDRIVE + HOMEPATH, USERPROFILE, and the  
Personal special folder in that sequence. This is the same algorithm  
that is used in Ruby 1.9.

- 

```
Added implementation of `, exec and system to KernelOps.cs

```

- 

```
Enabled mspec / rubyspec to run by cleaning up default.mspec.rb, 

```

runfirst.cmd. Enables support for specifying the class _and_ method when  
running tests / regressions / baselining.

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [July 9, 2008, 7:27am UTC](https://www.ruby-forum.com/t/code-review-fixes-2/142556/2 "2008-07-09T07:27:47Z")

</div>

I’ve included Srivatsn since he will be covering for me when I’m on  
paternity leave.

Context.rb:  
None of the changes in context are using the -B option, so they aren’t  
using the default.mspec, unless, perhaps, you have it in the default  
search locations for mspec.

I’ve been looking more, and running via mspec, instead of the direct  
mspec-{ci,tag,run} commands also can affect which specs run. Some specs  
are guarded for if the runner is MSpec or RSpec, using that env.  
Variable (see mspec/lib/mspec/guards/runner.rb). I’d also expect more  
config to move into mspec, so we probably want to keep using it.

I also wonder why you removed the common command that removed repetition  
when preparing the command line.

Default.mspec.rb:  
I purposely left core and lang active in default.mspec to allow one  
command to run all passing tests. Do we want that changed? Target  
should also stay in to allow mspec to be run without requiring the -t  
target.

Rakefile:  
I don’t see the point in changing path\_to\_ir to iruby, isn’t path\_to\_ir  
already a string?

Runfirst.cmd:  
I thought it made sense to keep the config file both in ~, any reason  
why not?

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [July 9, 2008, 7:34am UTC](https://www.ruby-forum.com/t/code-review-fixes-2/142556/3 "2008-07-09T07:34:48Z")

</div>

Comments inline:

Thanks,  
-John

> Context.rb:  
> None of the changes in context are using the -B option, so they aren’t  
> using the default.mspec, unless, perhaps, you have it in the default  
> search locations for mspec.

Thanks for the catch.

> I’ve been looking more, and running via mspec, instead of the direct  
> mspec-{ci,tag,run} commands also can affect which specs run. Some specs  
> are guarded for if the runner is MSpec or RSpec, using that env.  
> Variable (see mspec/lib/mspec/guards/runner.rb). I’d also expect more  
> config to move into mspec, so we probably want to keep using it.

Not sure what this means? I had trouble getting the target stuff working  
in the past, which is why I’m using the direct commands.

> I also wonder why you removed the common command that removed  
> repetition when preparing the command line.

This wound up making it much more difficult to tell at a glance what was  
going on - there was magic inside of that method. I prefer to see the  
entire command at a glance which is what the original solution did.

> Default.mspec.rb:  
> I purposely left core and lang active in default.mspec to allow one  
> command to run all passing tests. Do we want that changed? Target  
> should also stay in to allow mspec to be run without requiring the -t  
> target.

I don’t think that enabling core at this point makes sense since none of  
the test running infrastructure lets you selectively ‘run’ a particular  
core test like we do with the libs. I think we should think some more  
about this before we enable the core specs.

> Rakefile:  
> I don’t see the point in changing path\_to\_ir to iruby, isn’t path\_to\_ir  
> already a string?

iruby is a local variable, path\_to\_ir is a method call. iruby will be  
used in the future to pass a -X:Interpret flag to run the specs under  
Tomas’ beauty new interpreter.

> Runfirst.cmd:  
> I thought it made sense to keep the config file both in ~, any reason  
> why not?

Because the relative paths inside of default.mspec wouldn’t line up  
otherwise. Besides, the default locations of the rubyspec, mspec, and  
ironruby-tags projects all assume a dev subdirectory under home.

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [July 9, 2008, 7:43am UTC](https://www.ruby-forum.com/t/code-review-fixes-2/142556/4 "2008-07-09T07:43:13Z")

</div>

> Thanks,
> 
> > 
> 
> > I’ve been looking more, and running via mspec, instead of the direct  
> > mspec-{ci,tag,run} commands also can affect which specs run. Some  
> > specs  
> > are guarded for if the runner is MSpec or RSpec, using that env.  
> > Variable (see mspec/lib/mspec/guards/runner.rb). I’d also expect more  
> > config to move into mspec, so we probably want to keep using it.
> 
> Not sure what this means? I had trouble getting the target stuff  
> working in the past, which is why I’m using the direct commands.

Mspec sets the environment variable MSPEC\_RUNNER to equal 1. There is a  
guard that prevents specs from running if MSPEC\_RUNNER is set, or maybe  
vice versa. The point is to be able to disable specs that may be  
affected by the RSpec framework. In addition, the logic for running  
parallel is in mspec. Finally, I expect that future configuration might  
be moved to the mspec script. The -t option should work fine if you pass  
it a full path. I’ve only had problems passing things with the  
shortcuts, due to path issues. In addition, putting it back in the  
default.mspec file should make it a moot point, that’s how we were  
running it yesterday. If you are seeing problems with the default.mspec  
file, you may need to make sure that the file ends in .mspec, the runner  
checks for this.

> > I also wonder why you removed the common command that removed  
> > repetition when preparing the command line.
> 
> This wound up making it much more difficult to tell at a glance what  
> was going on - there was magic inside of that method. I prefer to see  
> the entire command at a glance which is what the original solution did.

Fair enough. I was just trying to avoid changing in 4 places, that’s  
part of why I ended up with a half done refactor.

> some more about this before we enable the core specs.  
> I think you mean lang. Core is the std lib tests. Lang is selectively  
> runnable via the mspec:lang task, which I use due to more  
> configurability (method and runner selection).

> > Rakefile:  
> > I don’t see the point in changing path\_to\_ir to iruby, isn’t  
> > path\_to\_ir  
> > already a string?
> 
> iruby is a local variable, path\_to\_ir is a method call. iruby will be  
> used in the future to pass a -X:Interpret flag to run the specs under  
> Tomas’ beauty new interpreter.

Sounds good. I just wanted to know.

> > Runfirst.cmd:  
> > I thought it made sense to keep the config file both in ~, any reason  
> > why not?
> 
> Because the relative paths inside of default.mspec wouldn’t line up  
> otherwise. Besides, the default locations of the rubyspec, mspec, and  
> ironruby-tags projects all assume a dev subdirectory under home.

Ok.
