This project is archived and is in readonly mode.

#2576 ✓stale
Jarl Friis

integration test fails on complex forms updating multiple models

Reported by Jarl Friis | April 28th, 2009 @ 03:12 PM | in 3.x

Hi.

The problem is described on http://groups.google.com/group/r...

I figured out that it was a problem in the Mock request generated by the rails framework.

I have made a patch.

The patch include tests that demonstrates the problem plus code that fixes the problem.

Comments and changes to this ticket

  • Jarl Friis

    Jarl Friis April 28th, 2009 @ 03:16 PM

    Oh, there was some outcommented debug code in my diff, here is a more clean diff

    Sorry.

  • Jarl Friis

    Jarl Friis April 29th, 2009 @ 07:54 AM

    • Tag changed from forms, integration_test, multipart to forms, integration_test, multipart, params, patch, patched
  • Jarl Friis

    Jarl Friis April 29th, 2009 @ 07:55 AM

    • Tag changed from forms, integration_test, multipart, params, patch, patched to fields_for, fix, forms, integration_test, multipart, params, patch, patched
  • Jarl Friis

    Jarl Friis April 29th, 2009 @ 08:16 AM

    I have now made a patch according to "Contributing to Rails" article https://rails.lighthouseapp.com/...

    Here it is.

  • josh

    josh April 29th, 2009 @ 01:37 PM

    • Assigned user set to “josh”
  • josh

    josh May 5th, 2009 @ 02:14 AM

    • State changed from “new” to “resolved”

    This should be fixed in edge. If its still not working for you, please create a ticket on the rack lighthouse.

  • Jarl Friis

    Jarl Friis May 5th, 2009 @ 10:40 AM

    Seems like current edge requires newer rubygems than I have on my Intrepid. I'll take a look at it when I upgrade my dev-box to Jaunty. I can at least see that edge does no longer contain the failing code, it has probably moved to the rack project.

    However, looking at rack edge source code, it seems that Utils::Multipart.build_multipart in rack/lib/rack/utils.rb contains the essential fix.

  • Jarl Friis

    Jarl Friis September 7th, 2009 @ 11:04 AM

    In case someone is interested in a patch for Rails 2.3.3, here it is.

    Jarl

  • Jarl Friis

    Jarl Friis September 16th, 2009 @ 08:32 AM

    I wonder why no one applies this patch to the 2.3.x branch.

    I have to do this manually every time I upgrade rails (now to 2.3.4)

    This is unpractical.

    Jarl

  • Jarl Friis

    Jarl Friis January 18th, 2010 @ 01:46 PM

    Joshua, I doubt that this is fixed in Edge (as of May 2009). SInce the problem still exists in 2.3.5. Could you please have a look at it again. Otherwise please guide as to how I should show a failing unit test in edge.
    In what file shoud I add a test? Or in what directory should I add a file?
    then what (rake) command should I run to see test results.

    I here attach a patch for 2.3.5

    Jarl

  • Jarl Friis

    Jarl Friis January 18th, 2010 @ 06:58 PM

    I have now made a patch (according to contributor guide: https://rails.lighthouseapp.com/projects/8994/sending-patches). The patch is against the 2-3-stable branch.

    The patch includes two unit tests that demonstrates the problem plus the fix of course.

    Please apply this patch against 2.3 branch, will you?

    Jarl

  • Jon Yurek

    Jon Yurek June 4th, 2010 @ 09:46 PM

    The above patch did not work for us. I've modified it a bit and it causes the app tests that were previously failing to pass. This patch is based on Jarl's. It applies cleanly to 2-3-stable.

  • Jarl Friis

    Jarl Friis June 7th, 2010 @ 11:05 AM

    Good to see there is some interest in this ticket.

    Just of curiousity: Which "above patch" did not work? I have supplied several patches to this ticket. You say that your patch is based on mine (but not which one), and at the same time "the above patch did not work" while all of my patches are above your comment :-)

    Another question: which branch is you patch against?

    Note further that this ticket is marked "resolved" because Joshua believes this is fixed in edge. However it has not been verified by anyone yet, and even if it was solved I would love to see tests (in edge) that ensured not introducing the bug again.

    Jarl

  • José Valim

    José Valim July 17th, 2010 @ 07:26 PM

    • Milestone changed from 2.x to 2.3.9
    • State changed from “resolved” to “open”
    • Assigned user changed from “josh” to “José Valim”
    • Importance changed from “” to “Low”

    Great work guys. I will apply this on 2-3-stable. Can someone provide a test case for master (in any case, I saw this issue being fixed in rack-test)? Thanks!

  • Nick Quaranto

    Nick Quaranto July 17th, 2010 @ 07:28 PM

    José, Yehuda just applied it to 2-3-stable. It's fixed in master as far as I know because rack's multipart params parsing is a lot smarter. A test case for it would be a really good idea though.

  • José Valim

    José Valim July 21st, 2010 @ 02:34 PM

    • Milestone cleared.
    • Tag changed from fields_for, fix, forms, integration_test, multipart, params, patch, patched to bugmash, fields_for, fix, forms, integration_test, multipart, params, patch, patched
  • José Valim

    José Valim July 26th, 2010 @ 01:28 PM

    • Milestone set to 3.x
  • Jarl Friis

    Jarl Friis August 3rd, 2010 @ 11:26 AM

    Actually the patch provided April 29th, 2009 @ 08:16 AM (https://rails.lighthouseapp.com/projects/8994/tickets/2576/a/116520...) contains a test. However the following diffs I have submitted is without this test.

    The test probably needs to be updated to the latest 2-3-stable branch.

    Why is this changed from 2.3.9 to 3.x?

    Jarl

  • José Valim

    José Valim August 3rd, 2010 @ 11:38 AM

    Jarl, it was applied on 2.3.9. We just need a test to be added to Rails 3.0 to ensure we won't have regressions.

  • Jarl Friis

    Jarl Friis August 3rd, 2010 @ 11:49 AM

    @José: Thanks for the information, looking forward to 2.3.9. Still I hope you can use the test in the patch to make a test in 3.x, but it sounds like the architecture changes requires a new test to be written.

  • José Valim

    José Valim September 2nd, 2010 @ 10:58 AM

    • State changed from “open” to “stale”
  • Jarl Friis

    Jarl Friis September 2nd, 2010 @ 12:05 PM

    Why is state changed to stale? and what does it mean?

  • José Valim

    José Valim September 2nd, 2010 @ 12:12 PM

    I'm marking it as stale as I didn't get a patch back. I will gladly reopen it if one is provided.

  • Andrea Campi

    Andrea Campi October 11th, 2010 @ 07:24 AM

    • Tag changed from bugmash, fields_for, fix, forms, integration_test, multipart, params, patch, patched to bugmash, fields_for, fix, forms, integration_test, multipart, params, patch

    bulk tags cleanup

  • Jarl Friis

    Jarl Friis November 8th, 2010 @ 08:12 AM

    I have tested this in 2.3.9, and it seems fixed there.

    I consider this bug as resolved, fix released.

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer November 8th, 2010 @ 08:26 AM

    • Tag cleared.

    Automatic cleanup of spam.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages