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

Ticket #4867 (closed defect: fixed)

Opened 4 years ago

Last modified 2 years ago

[PATCH] request URI not updated with multiple calls of get in functional tests

Reported by: Alisdair McDiarmid <alisdair@randomoracle.org> Assigned to: David
Priority: normal Milestone:
Component: ActionPack Version: 1.2.3
Severity: normal Keywords: tiny request_uri needs_review
Cc:

Description

If get is called multiple times with one TestRequest object, the request URI is not updated. This makes some testing awkward: for example, testing multiple controllers in a loop, and asserting success with the current URI as a message.

This patch (test included) is one way around this. It makes a note of when the set_REQUEST_URI method is called, to allow manual overriding of the REQUEST_URI environment variable. Otherwise, it always resets the value to the appropriate value for the most recent set of parameters.

Attachments

update_request_uri_with_multiple_test_requests.patch (2.3 kB) - added by Alisdair McDiarmid <alisdair@randomoracle.org> on 04/24/06 21:47:30.
4867.1-2-stable.patch (2.4 kB) - added by mpalmer on 08/15/07 01:49:19.
Rebased against r7277 (1-2-stable)

Change History

04/24/06 21:47:30 changed by Alisdair McDiarmid <alisdair@randomoracle.org>

  • attachment update_request_uri_with_multiple_test_requests.patch added.

05/10/06 21:48:50 changed by anonymous

  • keywords set to tiny.

12/19/06 14:39:21 changed by jgarber

  • keywords changed from tiny to tiny request_uri.

I'm running into this problem too. Patch looks good; please apply it soon!

05/25/07 07:02:26 changed by danger

This is also a useful patch - could you please update it against the current trunk?

05/25/07 20:05:26 changed by alisdair

Sure I could! What's the point, though? No-one will bother to commit it.

05/25/07 20:15:00 changed by danger

you'd be surprised how high the odds are if it's a clean patch that is well-written and doesn't add/change anything it doesn't have to and is well-tested.

Most of the communal sense that patches don't get applied comes from the vast array of bad/incomplete patches.

08/14/07 04:51:42 changed by mpalmer

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

I can't make the supplied test case fail with current edge (r7309). Alastair, could you take a look at the current edge and see whether you can still make it fail, and if so, upload a new test case and reopen this ticket?

I know the patch submission process is a pest, but you're more likely to get a working patch submitted than a dodgy one... (grin)

08/14/07 05:05:08 changed by mpalmer

Oh, as a further comment: although it doesn't apply cleanly, the patch does show and fix a valid problem in Rails 1.2.3. So the bug has definitely existed in the past, and the patch does fix that bug. What happened in Edge, though, I don't know...

08/15/07 01:49:19 changed by mpalmer

  • attachment 4867.1-2-stable.patch added.

Rebased against r7277 (1-2-stable)

08/15/07 01:51:26 changed by mpalmer

  • status changed from closed to reopened.
  • version changed from 1.1.1 to 1.2.3.
  • resolution deleted.

I've rebased this patch against the 1-2-stable branch, and verified that the test case identifies a problem and the live code fixes it. I think it's definitely a candidate for fixing in the stable branch; it's certainly bitten me in the butt (with the form_test_helper plugin), and it isn't particularly intrusive.

08/15/07 03:46:22 changed by danger

+1 This looks good!

Test case fails with current code, passes with patch. I'll leave it to someone who's more familiar with this are of code to decide if there might be a slightly more elegant solution but it looks great to me.

08/15/07 05:13:31 changed by mpalmer

  • keywords changed from tiny request_uri to tiny request_uri needs_review.

Adding keyword at the suggestion of nzkoz on the rails-core list.

08/15/07 05:35:25 changed by nzkoz

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

(In [7320]) Ensure TestRequest#request_uri returns the right value when doing multiple GETs. Closes #4867 [alisdair@randomoracle.org, mpalmer]