Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Session Mocking in Sinatra Test #427

Closed
leifg opened this Issue Sep 5, 2012 · 3 comments

Comments

Projects
None yet
2 participants

leifg commented Sep 5, 2012

Hello,

I'm not sure if this is the right project, but as it can be fixed in rack itself I assume it's best to submit the issue here.

I am currently on a Sinatra project where I use the Rack::Session::Pool.

For my functional tests I need to have certain objects in the session. The sinatra documentation tells me I can do that by passing the parameters in the Mock Request Methods (http://www.sinatrarb.com/testing.html#racktests_mock_request_methods)

In my example it looks like this:

get "/", {}, {"rack.session" => {'session' => "data"}}

However, the data I pass to the "rack.session" will be ignored. The result in the rack_env looks like this
... "rack.session"=>#<Rack::Session::Abstract::SessionHash:0x16c76bc not yet loaded> ...

I took a deeper look on how the session is built and found this snippet:

    def prepare_session(env)
      session_was                  = env[ENV_SESSION_KEY]
      env[ENV_SESSION_KEY]         = SessionHash.new(self, env)
      env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options)
      env[ENV_SESSION_KEY].merge! session_was if session_was
    end

Which is supposed to take care of adding the extra parameters to the session.

The problem here is:

The merge! method is not implemented in Rack::Session::Abstract::SessionHash, so it is basically a no-op.

It can be easily monkey patched by doing:

class Rack::Session::Abstract::SessionHash < Hash
  def merge! hash
    load_for_write!
    super(stringify_keys(hash))
  end
endRack::Session::Abstract::SessionHash

But I'm not sure if adding the merge method to Rack::Session::Abstract::SessionHash will cause harm on other places. If that is not the case, I would be glad to provide a pull request.

regards
Leif

@leifg leifg closed this Sep 5, 2012

@leifg leifg reopened this Sep 5, 2012

leifg commented Sep 5, 2012

I'm sorry I just saw that exactly that has been done in the current code base.

However I would suggest to stringify the keys as it is done in the update method. Otherwise there is no way a symbol key can be found.

leifg commented Oct 21, 2012

what's the status on this one?

Owner

raggi commented Nov 2, 2012

Fixed in 6b115c5. Thanks!

@raggi raggi closed this Nov 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment