Please change the subject to use imperative; e.g., "Refactor compiler specs to use 'let'"

hi @BanzaiMan not only let but also many other like be_nil inspite of nil and also match_array,expect so in commit message should I mention all these thanks

Done

Thanks you @BanzaiMan

cc @BanzaiMan @headius

If it is acceptable then I can proceed to modify many other rspec codes which are still written in older style

Thanks

Doesn't changing from the == matcher to match_array makes the tests weaker (they no longer assert on array order)? Wouldn't the eq matcher (which is the same as == but works with the new expect syntax) be the better choice?

My other concern is more to do with test readability: moving the array called "expected" all the way up to the top of the file makes it less clear what that assertion is doing.

Edit: Same goes for "base" being extracted to a let. It's no longer local to where it's being used, and it's not shared between examples so there doesn't seem to be any advantage to using a let.

Please look into Expected vs should for better rspec http://betterspecs.org/#expect yes all let for :base are used in local codes rajcybage@7dbb03e#L0R141 to rajcybage@7dbb03e#L0R154

Please look into the time taken by expect less than should

 Benchmark.bm do |x|
   it "adds an edge to newly created graph" do
    x.report("should"){@graph.edges.size.should be 0}

    x.report("expect"){expect(@graph.edges.size).to eq 0}
  end
 end

  $ rspec spec/ir/directed_graph/directed_graph_spec.rb
                 user     system      total        real
      should  0.004000   0.000000   0.004000 (  0.005000)
      expect  0.003000   0.000000   0.003000 (  0.003000)`

I'm not disagreeing with the switch from "should" to "expect" (though I wouldn't use that benchmark to justify it, but that's another story*). What I'm saying should be changed is:

1. Go back from "match_array" to "eq". match_array is the wrong assertion for these tests. The order of the array should be checked.
2. Get rid of those "let" blocks. They aren't being used to remove repetition of a variable, and moving the example data to the top of the file just makes it less clear what the test is asserting in both cases.

* For those interested in why I don't think those benchmark numbers are a good justification:

• Even if the numbers are correct, the difference in speed is minimal and unlikely to have a noticeable impact on spec performance.
• When the time taken the function being benchmarked is so small, you need to run the function many times and take an average of the time taken. There are too many things which could account for a 1ms difference in a function executing.