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...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

boombatower’s picture

Status: Active » Postponed

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.

pwolanin’s picture

FileSize
3.88 KB

here's an additional test derived from: http://drupal.org/node/117748#comment-754417

moshe weitzman’s picture

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.

boombatower’s picture

Project: SimpleTest » Drupal core
Version: 6.x-1.x-dev » 7.x-dev
Component: Code » simpletest.module
Status: Postponed » Needs work

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.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
3.79 KB

ok, re-rolled the above test now that we have a location in includes/tests

pwolanin’s picture

Component: simpletest.module » forms system
boombatower’s picture

pwolanin’s picture

@boombatower - does another test matter? The patch there for moving tests will probably not be used but rather just cvs rm, cvs add.

boombatower’s picture

Not really, it can be re-rolled. Wasn't stating that as a stopper.

catch’s picture

Status: Needs review » Reviewed & tested by the community

38 passes, 0 fails, 0 exceptions

Test looks fine to me as well, so RTBC.

Dries’s picture

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?

pwolanin’s picture

@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.

pwolanin’s picture

FileSize
3.8 KB

Here's the same patch except for a re-worded code comment.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

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.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.