merb

Comments and changes to this ticket

• 

  flazz July 25th, 2008 @ 07:01 PM

  the only way i can get a precise behavior spec is

    it "should never settle for anything 2" do
      lambda { dispatch_to(Contr, :index) }.should raise_error Merb::ControllerExceptions::NotAcceptable
    end
  
  

  i think .should be_client_error is much cleaner

• 

  Ben Burkert July 25th, 2008 @ 08:26 PM

  looks like the dispatch_to method doesn't have a begin..rescue statement that calls the exception controller like here: http://github.com/wycats/merb-co...

  Adding this bit of code would probably make the ".should be_client_error" specs working, but it could also change the behavior of people's existing specs. Personally, I like the lambda style better anyways.

• 

  Michael Klishin (antares) August 2nd, 2008 @ 06:18 PM

  • → Milestone changed from “” to “0.9.4”
  • → State changed from “new” to “open”
  • → Assigned user changed from “” to “Yehuda Katz (wycats)”
• 

  Yehuda Katz August 7th, 2008 @ 12:04 AM

  I think we should change the dispatch_to helper to actually go through the dispatcher, which will catch the error and render what will actually be rendered in real life.

  I will take up that task once the new dispatcher is merged in :)

• 

  Michael Klishin (antares) August 7th, 2008 @ 01:25 AM

  Yehuda,

  Does it mean new dispatch_to will use router?

• 

  Michael Klishin (antares) August 12th, 2008 @ 08:05 PM

  • → Milestone changed from “0.9.4” to “0.9.5”
• 

  Michael Klishin (antares) August 23rd, 2008 @ 10:56 PM

  • → Milestone changed from “0.9.5” to “0.9.6”
• 

  Michael Klishin (antares) September 9th, 2008 @ 02:19 AM

  • → Milestone changed from “0.9.6” to “0.9.7”
• 

  Michael Klishin (antares) September 14th, 2008 @ 02:00 AM

  • → Milestone changed from “0.9.7” to “0.9.9”
• 

  Yehuda Katz (wycats) September 14th, 2008 @ 03:39 AM

  @antares Yes, that's what it means. In my view, there are two kinds of tests:

  1) tests that want to hit the results of a certain generated url(). This is when the only way to get to a certain URL is via, for instance, a form submission, which has an URL generated as an action. In that case, you should do something like request(url(...)), which would match up with the url() used in your views. This would alert you if you changed your routes, and the generated url no longer points to the right place.

  2) tests that want to hit an explicit URL. For instance, if you use URLs as part of a public API, or you want to make sure content indexed by Google stays in the same place. In that case, you should do something like request(...). This would alert you if route changes no longer point to the correct URL.

  There is no case where you want to test a specific controller and action, because there's no external interface to get to that controller and action. For instance, if you completely rename a controller, and modify the route pointing to it, no tests should fail!

  Remember: ideal tests fail when there are bugs, and don't fail when there are no bugs. Or shorter: ideal tests fail only when there are bugs.

• 

  Michael Klishin (antares) September 14th, 2008 @ 03:54 AM

  But we have helpers that use router and that do not. One picks what fits the situation. I personally do not use mocks for objects and try to make tests use "real things" but I still like separation of routing specs and controller specs.

  I would leave dispatch_to as it is.

• 

  Yehuda Katz (wycats) September 14th, 2008 @ 04:37 AM

  @antares I have no problem with leaving around dispatch_to for back-compatibility.

  However, the approach of testing that a certain input produces a certain output, and not worrying about the internals of how the input gets to the output produces significantly less brittle tests.

  Of course, you have to figure out what the "input" actually means. Above, I said that there were two kinds of input: (a) cases where the user never types in an URL, but the URL comes from using the url() helper in your views and (b) cases where the user types in an URL, because it's part of a public API, or is indexed on Google.

  In case (a) you can test something like request(url(...)), which will confirm that the use of that URL in your views still points to the same place. If you change your routes, the tests will fail, which will mean that you need to go back and change the url() calls in your views. In case (b) you test the URL directly, because you want to confirm that the URL that people are using (again, either via a public API or because it's indexed by Google) isn't changing.

  At no point do you really care exactly what the name of the controller or action is. I've whipped up a quick gist to demonstrate: http://gist.github.com/10680

  Again, this goes back to the idea of regressions. You want your test suite to fail when there are regressions, and pass when there aren't. The gist demonstrates a case where no regression happened but a test failed anyway.

• 

  Michael Klishin (antares) September 14th, 2008 @ 04:38 AM

  After some arguing at the IRC I more or less convinced and going to try this "correct" approach :)

• 

  Matthijs Langenberg September 15th, 2008 @ 02:06 PM

  Also ran into this. :-). I tried to do integration style test, where I used invalid credentails to check the return value. Doesn't Rack provide a way to do a real request? So instead of mongrel passing a request to Rack, we do it directly.

• 

  flazz September 15th, 2008 @ 04:19 PM

  Some questions about all of this. Just for my information to get a better handle on what is going on (please humor me guys):

  Dispatch to is to be deprecated? If so what is the recommended way to spec a controller interaction? I for one have no problem updating my specs to better specs. I was just going off what was possible at the time (or what I thought was @ 0.9.3) If there is a better way I'd love to switch to that.

  To properly use this convenience matcher (or matchers like it) on a controller would break the shearing layer and start to get into routing territory? If specs span shearing layers they should be more of an acceptance test and emulate a real world use case expectation right? It seems that in the case of the 'request specific URL, get expected response' pattern would be in this category and not a controller spec.

• 

  Michael Klishin (antares) September 15th, 2008 @ 06:05 PM

  It's not gonna be deprecated, just a technique that is considered a better approach (with routing and URL request using get, post, etc) has no this problem.

• 

  Matthijs Langenberg September 16th, 2008 @ 03:57 PM

  • → Tag changed from “controller rspec spec” to “controller rspec spec”

  I really hate having a failing spec when the app actually works. In this case this is caused by duplicate dispatcher logic in Merb::Test::RequestHelper. That's why I tried looking for a way to do requests at the highest level (Rack application), and it turned out to be quite easy.

  
  # invoked using merb -i
  class Test < Application
    def index
      'Hello There!'
    end
  end
  
  include Merb::Test::RequestHelper
  
  app = Merb::Rack::Application.new
  request = fake_request({:request_uri => '/test'})
  response = app.call(request.env)
  p response
  [200, {"Date"=>"Tue, 16 Sep 2008 14:38:16 +0200", "Content-Type"=>"text/html; charset=utf-8"}, "Hello There!"]
  

  I'm going to write some helper functions for this and try it on the merb app I'm currently working on (also suffering from this bug).

  Can anyone tell me why there are already rack specific settings in the environment hash returned by fake_request. The current request helpers don't go through rack at all, did they in the past?

• 

  Yehuda Katz (wycats) September 19th, 2008 @ 11:37 AM

  @Matthijs Langenberg: I spent about 10 hours on my flight back to London on this and have made good progress :) I should have something to commit this weekend.

• 

  Matthijs Langenberg September 19th, 2008 @ 12:10 PM

  Yehuda, I'll check your code after the weekend. I already wrote some custom helpers to invoke Merb::Application.new with a merb request with good results.

• 

  Matthijs Langenberg September 29th, 2008 @ 10:45 AM

  @Yehuda Katz: I wanted to check out your changes but couldn't find your commits. Could you point me to them? Thx

  btw. I'm using this as a temporary solution:

  
      def rack_get(path, env)
        env[:method] = "GET"
        rack_request(path, env)
      end
  
      def rack_request(path, env)
        request = Merb::Request.new(Rack::MockRequest.env_for(path, env))
        Rack::MockResponse.new(*app.call(request.env))
      end
  
      def app
        @app ||= Merb::Rack::Application.new
      end
  

  This solves to problem of duplication between the real dispatcher and the test dispatcher. The side effect is that I need to look in test.log to see raised exceptions.

• 

  Michael Klishin (antares) October 7th, 2008 @ 12:02 PM

  • → Milestone changed from “0.9.9” to “1.0 Final”
• 

  Michael Klishin (antares) October 12th, 2008 @ 08:01 AM

  • → Tag changed from “controller rspec spec” to “controller rspec spec sprint triage”
  • → State changed from “open” to “invalid”
  • → Assigned user changed from “Yehuda Katz (wycats)” to “Michael Klishin (antares)”

  New testing code is committed and this issue can be easily "resolved" by using lambda { }.should_not raise_error. Closing.