Provide unit tests for includes/form.inc
pukku - March 7, 2008 - 18:20
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | forms system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
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...
| Attachment | Size |
|---|---|
| form_inc_tests.patch | 2.23 KB |
#1
We 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.
#2
here's an additional test derived from: http://drupal.org/node/117748#comment-754417
#3
If 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.
#4
Once 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.
#5
ok, re-rolled the above test now that we have a location in includes/tests
#6
#7
Would be nice if this went in first #268063: Move includes/tests to simpletest/tests & provide hidden .info propery.
#8
@boombatower - does another test matter? The patch there for moving tests will probably not be used but rather just cvs rm, cvs add.
#9
Not really, it can be re-rolled. Wasn't stating that as a stopper.
#10
38 passes, 0 fails, 0 exceptions
Test looks fine to me as well, so RTBC.
#11
I'm not sure about:
+ // Avoid output drupal warnings and messages into assert messages+ drupal_get_messages();
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?
#12
@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.
#13
Here's the same patch except for a re-worded code comment.
#14
pwolanin: 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.
#15
Automatically closed -- issue fixed for two weeks with no activity.