[Ironruby-core] Code Review: fixes-2

Jim Deville jdeville at microsoft.com
Wed Jul 9 01:41:26 EDT 2008

> -----Original Message-----
> From: John Lam (IRONRUBY)
> Sent: Tuesday, July 08, 2008 10:31 PM
> To: Jim Deville; ironruby-core at rubyforge.org; Oleg Tkachenko; Tomas
> Matousek
> Cc: Srivatsn Narayanan
> Subject: RE: Code Review: fixes-2
> Comments inline:
> Thanks,
> -John
> > -----Original Message-----
> > From: Jim Deville
> > Sent: Tuesday, July 08, 2008 10:21 PM
> > To: ironruby-core at rubyforge.org; John Lam (IRONRUBY); Oleg Tkachenko;
> > Tomas Matousek
> > Cc: Srivatsn Narayanan
> > Subject: RE: Code Review: fixes-2
> >
> > 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.
> 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.

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.

> > 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.

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.


More information about the Ironruby-core mailing list