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.
THE VERSION ON THIS IS INCORRECT -- THIS IS FOR CVS/HEAD/DRUPAL 7.x.
Attached is a patch to add some tests for form.inc.
This is a very initial version, which only tests two of the theme functions, and provides only the most basic tests at that. But it's a start...
Comment | File | Size | Author |
---|---|---|---|
#13 | fapi-test-231302-13.patch | 3.8 KB | pwolanin |
#5 | fapi-test-231302-5.patch | 3.79 KB | pwolanin |
#2 | TestCase117748-231302.patch | 3.88 KB | pwolanin |
form_inc_tests.patch | 2.23 KB | pukku | |
Comments
Comment #1
boombatower CreditAttribution: boombatower commentedWe haven't started unit testing officially yet.
We are working on how we want it done and any associated standards.
Once we begin then reroll your patch if necessary.
If you want to help finish functional testing you can write a test that hasn't already as seen on the list: http://groups.drupal.org/node/9408.
Comment #2
pwolanin CreditAttribution: pwolanin commentedhere's an additional test derived from: http://drupal.org/node/117748#comment-754417
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedIf we do decent test for poll module we will effectively have tested the obscure parts of fapi. other tests such as node edit assure the common stuff works ... even so, focused unit tests for fapi are always welcome.
Comment #4
boombatower CreditAttribution: boombatower commentedOnce the convention for include files is figured out this can be completed. http://groups.drupal.org/node/11020 Which should just decide on where to but include file tests. So far I believe it is includes/tests/[name_of_file].test.
Otherwise we aren't going to do unit testing in the manor originally being planned so this work can commence.
These tests need updating and completion.
Comment #5
pwolanin CreditAttribution: pwolanin commentedok, re-rolled the above test now that we have a location in includes/tests
Comment #6
pwolanin CreditAttribution: pwolanin commentedComment #7
boombatower CreditAttribution: boombatower commentedWould be nice if this went in first #268063: Move includes/tests to simpletest/tests & provide hidden .info propery.
Comment #8
pwolanin CreditAttribution: pwolanin commented@boombatower - does another test matter? The patch there for moving tests will probably not be used but rather just cvs rm, cvs add.
Comment #9
boombatower CreditAttribution: boombatower commentedNot really, it can be re-rolled. Wasn't stating that as a stopper.
Comment #10
catch38 passes, 0 fails, 0 exceptions
Test looks fine to me as well, so RTBC.
Comment #11
Dries CreditAttribution: Dries commentedI'm not sure about:
First, the code comment reads a but funny. Second, is this the place to do it? What happens with previous messages? Is this something we need to move to the web test case file?
Comment #12
pwolanin CreditAttribution: pwolanin commented@Dries - agreed the comment could use re-wording.
Basically the problem is that any dsm() error message shows up as an exception, which is normally useful so that PHP errors, etc, show up as test exceptions. In this case, we are causing the errors as a deliberate part of the testing. So, I don't think we want to clear them for normal tests, and even here it would be ideal to be able to just clear the form-required errors.
Comment #13
pwolanin CreditAttribution: pwolanin commentedHere's the same patch except for a re-worded code comment.
Comment #14
Dries CreditAttribution: Dries commentedpwolanin: yes, some more studying of the code let me to believe the same thing. Thanks for clarifying the code comment -- that will prove useful for others. Committed to CVS HEAD.
Yay to more tests.
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.