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

Ticket #11091 (closed enhancement: fixed)

Opened 7 months ago

Last modified 6 months ago

[PATCH] Allow File Uploads in Integration Testing

Reported by: RubyRedRick Assigned to: core
Priority: normal Milestone: 2.x
Component: ActiveRecord Version: edge
Severity: normal Keywords: verified
Cc:

Description

This is an attempt to answer this 2 year old ticket http://dev.rubyonrails.org/ticket/4635

The attached patch, with test case, adds automatic detection of one or more parameters to post and put requests during integration testing which contain an instance of TestUploadedFile, and makes the request multipart and properly marshals the uploaded files.

Attachments

multi-part-integration.diff (4.2 kB) - added by RubyRedRick on 02/12/08 20:12:51.
integ_upload_diff (6.1 kB) - added by RubyRedRick on 02/19/08 16:32:17.
New patch with modified testcase
integ_upload_post_11163.diff (4.5 kB) - added by RubyRedRick on 02/24/08 20:42:25.

Change History

02/12/08 20:12:51 changed by RubyRedRick

  • attachment multi-part-integration.diff added.

02/13/08 15:07:28 changed by pelargir

+1

02/13/08 17:20:45 changed by pat_maddox

+1, lovely

02/17/08 03:46:02 changed by aiwilliams

+1, PLEASE ADD THIS!

02/17/08 05:03:39 changed by chuyeow

  • keywords set to verified.

02/17/08 08:49:07 changed by nzkoz

I don't really like the way you're using an Exception as a 'goto' here? Could you restructure the code a little so it's not required?

02/17/08 13:52:45 changed by RubyRedRick

nzkoz,

I don't understand the objection, it's not a goto, this is actually the cleanest way to handle the situation. Having an uploaded file is relatively rare, so handling the occurrence as an exception, rather than either greatly complicating the main case, or doing separate pass over the parameters to determine if a multipart form is needed seems much messier.

02/18/08 00:38:44 changed by nzkoz

It's definitely an irregular occurrence but not sure it qualifies as an exceptional one. Either way the patch as it currently exists seems to stomp over the other tests?

http://pastie.caboo.se/153537

02/18/08 00:39:24 changed by nzkoz

Also, thanks! I'd love to have this functionality :)

02/18/08 03:49:23 changed by RubyRedRick

OK, the test case problem appears to be a problem with my testcase, NOT the operational patch.

I'll try to figure out how to fix that.

02/19/08 16:32:17 changed by RubyRedRick

  • attachment integ_upload_diff added.

New patch with modified testcase

02/19/08 16:40:36 changed by RubyRedRick

Opened #11163 with subset of the attached patch to fix the monkeypatched code in test/controller/integration_test.rb

02/24/08 20:42:25 changed by RubyRedRick

  • attachment integ_upload_post_11163.diff added.

02/24/08 20:43:14 changed by RubyRedRick

New patch compatible with changeset http://dev.rubyonrails.org/changeset/8899

This new patch simply removes the duplicated change to fix the monkeypatched code in the existing integration test test, which was adopted in slightly modified form in changeset 8899.

Again, the functional code in the initial patch is unchanged. The only changes since the initial patch have been cleaning up the testcase to fix isolation issues.

03/03/08 06:42:28 changed by nzkoz

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

(In [8978]) Allow file uploads in Integration Tests. Closes #11091 [RubyRedRick]