Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7372 (closed defect: fixed)

Opened 2 years ago

Last modified 7 months ago

[PATCH] TestSession should support indifferent access

Reported by: jean.helou Assigned to: core
Priority: normal Milestone: 1.2.7
Component: ActionPack Version: edge
Severity: major Keywords: verified test session indifferent
Cc: me@julik.nl, dharana@dharana.net, djwonk, tarmo

Description

In the CGIRequest, I can access HTTP headers using request.env[], but in a testcase, the HTTP headers which are not interpreted by rails seem to be removed from the env hash.

I have a controller with an action which relies on HTTP_ACCEPT_LANGUAGE to determine the language to be used to render the page, however I can't write tests for a language other than the default because the HTTP_ACCEPT_LANGUAGE header doesn't reach my controller when added to a TestRequest.

I have a failing testcase both in the form of a pastie at http://pastie.caboo.se/34430 and in the form of a tgz file

this fails against rails 1.2.1

Attachments

pipocontroller.tgz (53.6 kB) - added by jean.helou on 01/25/07 12:59:59.
a complete project allowing you to run the failing test
test_session_indifferent.diff (1.5 kB) - added by julik on 01/28/07 07:05:38.
Patch for the actual bug
fix_indifferent_access_for_session_batch_assignment.diff (1.7 kB) - added by manfred on 01/29/07 10:10:25.
test_session_indifferent_fixed_for_session_args.diff (3.0 kB) - added by mhackett on 03/07/07 17:57:40.
New version of patch (same as [6086]) plus fix for bug introduced with session arguments to process method.

Change History

01/25/07 12:59:59 changed by jean.helou

  • attachment pipocontroller.tgz added.

a complete project allowing you to run the failing test

01/28/07 07:05:38 changed by julik

  • attachment test_session_indifferent.diff added.

Patch for the actual bug

(follow-up: ↓ 2 ) 01/28/07 11:46:16 changed by bitsweat

By the way, HTTP_ACCEPT_LANGUAGE is the name of the CGI environment variable created for the Accept-Language header.

(in reply to: ↑ 1 ) 01/28/07 11:52:07 changed by bitsweat

Replying to bitsweat:

By the way, HTTP_ACCEPT_LANGUAGE is the name of the CGI environment variable created for the Accept-Language header.

Ah, nevermind. You can pass the 'real' http header with integration tests but must use the CGI env var in functionals.

01/28/07 18:07:47 changed by julik

  • cc set to me@julik.nl.
  • keywords changed from test request headers to test session indifferent.
  • severity changed from normal to major.
  • summary changed from ActionController::TestRequest doesn't support custom HTTP Headers to [PATCH] TestSession should support indifferent access.

Fix and test case attached. I think this is a major problem because it can lead to tests silently passing when they shouldn't

01/28/07 18:18:42 changed by bitsweat

  • milestone changed from 1.x to 1.2.

01/28/07 18:24:47 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.

(In [6086]) TestSession supports indifferent access so sessionfoo? == session[:foo] in your tests. Closes #7372.

01/28/07 18:26:13 changed by bitsweat

(In [6087]) Merge [6086] from trunk. References #7372.

01/29/07 10:10:05 changed by manfred

  • status changed from closed to reopened.
  • resolution deleted.

This patch breaks batch assignment for the session in for example: 

get :show, {:id => 1}, {:user_id => 1}

Because the attributes in TestSession now expect all keys to be strings. See the added patch for a solution and testcase.

01/29/07 10:10:25 changed by manfred

  • attachment fix_indifferent_access_for_session_batch_assignment.diff added.

(follow-up: ↓ 13 ) 02/08/07 00:46:41 changed by orangechicken

This is a killer for functional tests with sessions. Too bad this made it into 1.2.2... shall we see 1.2.3 soon?

02/11/07 18:13:57 changed by dharana

  • cc changed from me@julik.nl to me@julik.nl, dharana@dharana.net.

It's also a killer for me.

02/11/07 19:30:20 changed by bitsweat

The changes were rolled back. Look for 1.2.3 shortly.

02/11/07 21:36:13 changed by dharana

Thank you for the info bitsweat.

02/18/07 22:09:52 changed by orangechicken

Hey Bitsweat, just a tiny nag: Any word on 1.2.3?

(in reply to: ↑ 8 ) 02/21/07 00:19:34 changed by Arsen7

Replying to orangechicken:

This is a killer for functional tests with sessions. Too bad this made it into 1.2.2... shall we see 1.2.3 soon?

A quick fix for this: 

--- actionpack-1.13.2/lib/action_controller/test_process.rb-org 2007-02-21 00:52:50.000000000 +0100
+++ actionpack-1.13.2/lib/action_controller/test_process.rb     2007-02-21 00:49:53.000000000 +0100
@@ -279,7 +279,7 @@

     def initialize(attributes = nil)
       @session_id = ''
-      @attributes = attributes
+      @attributes = attributes && attributes.stringify_keys
       @saved_attributes = nil
     end

Forgive me for not providing any tests for this, but this worked for my functional tests and it's very late tonight ;-)

03/05/07 05:26:52 changed by djwonk

  • cc changed from me@julik.nl, dharana@dharana.net to me@julik.nl, dharana@dharana.net, djwonk.

03/07/07 17:57:40 changed by mhackett

  • attachment test_session_indifferent_fixed_for_session_args.diff added.

New version of patch (same as [6086]) plus fix for bug introduced with session arguments to process method.

03/07/07 18:05:36 changed by mhackett

The new patch incorporates Arsen7's very concise fix and a test for it (one that failed with the previous patch and code in release 1.2.2). Also see ticket #7632 -- reports the problem we're talking about here, so this should close that ticket as well.

08/24/07 21:46:34 changed by tarmo

  • cc changed from me@julik.nl, dharana@dharana.net, djwonk to me@julik.nl, dharana@dharana.net, djwonk, tarmo.

mhackett, patches should not include changelog entries, these are only meant to be written by the one commiting the patch. Though in this case I'm assuming you included it to make sure everyone involved gets credit which I think is fine.

Anyway, the test does apply, tests run fine though perhaps it would be better to use 

@attributes = attributes.stringify_keys unless attributes.nil?
instead of
@attributes = attributes.nil? ? nil : attributes.stringify_keys

+1

08/25/07 00:11:33 changed by julik

+1

01/23/08 19:04:34 changed by jeff

+1

01/27/08 21:10:51 changed by juanjo.bazan

  • keywords changed from test session indifferent to verified test session indifferent.

02/02/08 05:32:47 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [8782]) TestSession supports indifferent access. Closes #7372.

02/02/08 05:34:32 changed by bitsweat

(In [8783]) Merge r8782 from trunk: TestSession supports indifferent access. References #7372.

02/02/08 05:37:25 changed by bitsweat

(In [8784]) Merge r8782 from trunk: TestSession supports indifferent access. References #7372.