Rspec expect syntax

[zirni]

Following some conversations of the dm2 team about the rspecs new expetation syntax. Are you interested to convert available specs to the new syntax? I can help with that.

[mbj]

@zirni Yes. IMHO this conversation would be greatly appreciated! cc @dkubb.

I wounder if you can do this semi automagically via parser undparser and some ast walking magic :D. But dont waste your time on this idea, I think unparser needs to be able to reproduce comments before.

[snusnu]

@zirni i think what @mbj meant is that help with the conversion would be greatly appreciated! ;) I'm sure that any one of us would pull in such changes almost immediately, for all of the relevant libs.

[mbj]

@snusnu Heh, yeah.

[zirni]

@mbj That's a good point. Will look for that :) @snusnu Will see what I can do :)

[dkubb]

@zirni I would surely accept this kind of patch to axiom, or any other gem I maintain including the axiom related gems. Thank you for the offer ;)

[mbj]

@zirni Heh, I'd also accept this patch for all my stuff :D, in case you need more work.

[dkubb]

@mbj re: your statement about doing it automagically using parser/unparser, all I did was combine ack and perl to rewrite the statements. It worked in about 90% of the cases, and I just fixed the others manually. I don't recall what I did offhand, but next time I do a conversion I'll add the commands in a gist and/or link to them here so others can use them for a conversion.

[mbj]

@dkubb In case we'd have a mutation covered library, and unparser would reproduce comments. Than we could verify test equivalency quite easily. But I think that tooling is not ready, currently.

[zirni]

Most of the specs using should in combination with a subject. An appropriate representation for

it { should be_true }

was suggested in here http://stackoverflow.com/questions/12260534/using-implicit-subject-with-expect-in-rspec-2-11?answertab=votes#tab-top

expect_it { to be_true }

Is that going in your preferred direction?

[dkubb]

@zirni I probably keep the it { should ... } syntax for now as long as it's the default approach. Even though that SO post was written by an rspec maintainer, I'd rather hold off changing from the defaults until they've made a change in rspec itself.

Elsewhere I would change things like foo.should == bar to expect(foo).to eq(bar).

Also in general we use the highest precision matcher when we can. This means we:

• Prefer #eql to #eq (uses #eql? on the expected object)
• Prefer #be to #eql (uses #equal? on the expected object)

The general idea is that we want to be as precise as possible. If the specs begin to fail because some behaviour changes we want to know immediately.. if previously we were returning self, and then started to return a new, but equivalent object we'd want the specs to tell us. If it was our intention then we should change the specs to be more precise, but if it was unintentional, then it points to a bug in the code that needs fixing.

[dkubb]

Also, here's a few commands I was using to convert the code in another project:

# convert mock(...) and stub(...) to double(...)
ack -l '(\s)(mock|stub)\b' spec/ | xargs perl -pi -e 's!(\s)(mock|stub)\b!$1double!g'

# convert foo.should eql(bar) to expect(foo).to eql(bar)
ack -l '^(\s*)(.+?)\.should(_not)?\b' spec | xargs perl -pi -e 's!^(\s*)(.+?)\.should(_not)?\b!$1expect($2).to$3!g'

In general I try to use the same regexps in the ack command as in the first match in the perl s!!! expression. I use ack without the -l switch to see what be matched, and then pipe those files into perl to modify.

Once I visually inspect the changes using git diff I'll use git add to add them and continue making incremental changes until the specs pass.

Feel free to suggest improvements or point out issues with these. I found they get me about 80 or 90% of the way, and I fix the rest by hand.

[yujinakayama]

Please consider using Transpec. :)

[zirni]

@yujinakayama looks very helpful. Thx.

Probably I won't convert method stubs from obj.stub(:valid? => true) toallow(obj).to receive(:valid?).and_return(true)` Do you have preferences? :)

[yujinakayama]

You can use --disable option to disable specific conversions.

$ transpec --disable allow_to_receive

[dkubb]

I use the allow(..) syntax in some work projects and I quite like it. The allow(...) are nice and symmetrical with the expect(...) statements.

[dkubb]

@yujinakayama that is much nicer than my ad-hoc approach, I'll have to try that out.

[mbj]

@dkubb It uses parser internally. Looks good to me. Maybe @yujinakayama could also use unparser internally? Unparser will not fit out of the box, but I expect it might help for some "uneasy to process cases".

[dkubb]

@yujinakayama btw, I made the change in https://github.com/dkubb/axiom/commit/fe9c616e7737687914f6198cf051d3774d0be5bc and transpec worked perfectly. Thanks!

[yujinakayama]

@dkubb You're welcome!

@mbj Unparser looks great, but Transpec's policy is “Keep original code style as much as possible” so I chose rewriting code manually. Anyway, thanks for the suggestion.

[mbj]

@yujinakayama I see. Just remember unparser might help you in similar domains ;)

[mbj]

@yujinakayama And BTW: Cudos for transpec, I'll port lots of my gems soon.