#2576 integration test fails on complex forms updating multiple models - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2576 ✓stale

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

You flagged this item as spam.
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.
You flagged this item as spam.
Jarl Friis April 29th, 2009 @ 07:54 AM
- Tag changed from “forms, integration_test, multipart” to “forms, integration_test, multipart, params, patch, patched”
You flagged this item as spam.
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”
You flagged this item as spam.
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.
- integration_testing_complex_forms.diff 6.7 KB
josh April 29th, 2009 @ 01:37 PM
- Assigned user set to “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.
You flagged this item as spam.
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.
You flagged this item as spam.
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
- rails-2_3_3.patch 748 Bytes
You flagged this item as spam.
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
You flagged this item as spam.
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
- rails-2_3_5.patch 748 Bytes
You flagged this item as spam.
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
- 2-3-stable_patch_for_2576.diff 4 KB
You flagged this item as spam.
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.
- 0001-fix-for-integration-tests-not-serializing-arrays.patch 5.4 KB
You flagged this item as spam.
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 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!
You flagged this item as spam.
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 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 July 26th, 2010 @ 01:28 PM
- Milestone set to “3.x”
You flagged this item as spam.
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 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.
You flagged this item as spam.
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 September 2nd, 2010 @ 10:58 AM
- State changed from “open” to “stale”
You flagged this item as spam.
Jarl Friis September 2nd, 2010 @ 12:05 PM
Why is state changed to stale? and what does it mean?
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.
You flagged this item as spam.
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
You flagged this item as spam.
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 November 8th, 2010 @ 08:26 AM
- Tag cleared.
Automatic cleanup of spam.