Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Lets add some Test....
Comment | File | Size | Author |
---|---|---|---|
#17 | fivestar.form-id-changes.patch | 877 bytes | ericduran |
#14 | 1136124-wrong-form_id-variable-14.patch | 568 bytes | ericduran |
#12 | 1136124-wrong-form_id-variable.patch | 583 bytes | msonnabaum |
#5 | 1136124-5.fivestar-tests.patch | 3.48 KB | ksenzee |
#1 | 0001-Adding-simpletest-basic-setup.patch | 1.37 KB | ericduran |
Comments
Comment #1
ericduran CreditAttribution: ericduran commentedJust started, nothing there yet just the skeleton.
Comment #2
ericduran CreditAttribution: ericduran commentedI have some local test written, need to update patch, still not ready and I'm waiting for the last critical issue to be fixed before we starting enforcing test.
Comment #3
ksenzeeI also have some local tests written. They're probably a lot the same. Should I go ahead and upload a patch, or wait?
Comment #4
ericduran CreditAttribution: ericduran commented@ksenzee feel free to upload a patch, I'm waiting for #1212930: Convert the fivestar widget to use the D7 AJAX framework to get in, because that's going to break every test :)
Comment #5
ksenzeeI just wrote a couple of tests that don't touch ajax yet for that very reason. Here they are. I suppose they might change when that issue gets in, but that issue needs tests anyway (which I am happy to write) so I don't think adding this to the codebase would hurt anything.
Comment #6
ericduran CreditAttribution: ericduran commented@ksenzee Thanks, I'm testing this now, This is great start., I need to send Acquia a gift basket ;)
Comment #7
ksenzeeHeh. Other way around, I think. You've put up with our patches stacked up higher than a Jenga game. :)
Comment #8
ericduran CreditAttribution: ericduran commentedSpeaking of patches :) This is now fixed. I committed the patch above with changes required to meet the new instance settings.
http://drupalcode.org/project/fivestar.git/commit/5028ee587df103f47e72c5...
I'm leaving this active for now while I add the ajax text.
Comment #9
ericduran CreditAttribution: ericduran commentedThis one text that the proper ajax command is returned -- http://drupalcode.org/project/fivestar.git/commit/2bb3771
Still need to test the data in the ajax command.
As always more test are always welcome. I'm going to keep this issue open for a while, so we can keep adding test and attributing it to this issue.
Comment #10
ericduran CreditAttribution: ericduran commentedClosing this issue out. I added another set of test. I think for now this issue isn't needed.
Comment #12
msonnabaum CreditAttribution: msonnabaum commentedOpening this backup since 4f5a61d59b475a6a13f7eeeabac0236ee4cb74bb points here.
The comment introduced says:
// TODO: why does passing in NULL works, yet passing in the actual form-id doesn't work.
I think that's because the old version was doing:
drupal_process_form($form_state['form_id'], $form, $form_state);
instead of:
I have no idea what's actually happening here, I just saw the php notice for $form_state['form_id'], which didnt exist, and would explain why it didnt work before.
Comment #13
ericduran CreditAttribution: ericduran commentedThanks, Even though the correct param should actually be $form['#form_id'] but they should be the same.
I'll fix this.
Comment #14
ericduran CreditAttribution: ericduran commentedFor the test-bot
Comment #15
ericduran CreditAttribution: ericduran commented@msonnabaum Thanks.
I committed your patch with proper attribution then my param change on top of it. I like doing things in the proper order ;-)
--
http://drupalcode.org/project/fivestar.git/commit/5291166
http://drupalcode.org/project/fivestar.git/commit/c1e58cd
Comment #16
ericduran CreditAttribution: ericduran commentedActually opening this back up. I'm not sure it's worth processing the form, just for the single element.
Comment #17
ericduran CreditAttribution: ericduran commentedYet another patch, this one takes a different approach and removes the entire form process.
I'll need to try and remember why this is happening but it seems like it could be cleaned up a bit.
Uploading patch to see if it passes the test.
Comment #18
ericduran CreditAttribution: ericduran commentedComment #19
ericduran CreditAttribution: ericduran commentedThis is now fixed.
--
http://drupalcode.org/project/fivestar.git/commit/19673ea159958c2d22e4ab...