Boolean field allows multiple options, then throws error on submit

at the field description where you define the key|value pairs

i notice following text:

"The possible values this field can contain. Enter one value per line, in the format key|label. The key is the value that will be stored in the database, and must be a numeric value. The label is optional, and the key will be used as the label if no label is specified."

think the best way is to check the correct formatting of user input
before saving the field.

try this path

try again

That one should apply. Additionally, checks that we have no more or no less than 2 values.

Let's get tests for this, please.

Yay, thanks for stepping in Matt !

+++ modules/field_ui/field_ui.test	20 Nov 2009 22:02:31 -0000
@@ -207,4 +207,41 @@ class FieldUITestCase extends DrupalWebT
+  function testBooleanOptions() {

The test should go in field/modules/list/list.test rather than in field_ui/field_ui.test.
This will probably warrant a separate testcase class, with its own setUp() inspired from the one in field_ui.test

+++ modules/field_ui/field_ui.test	20 Nov 2009 22:02:31 -0000
@@ -207,4 +207,41 @@ class FieldUITestCase extends DrupalWebT
+    //check missing option

Formatting + misses trailing point.

+ ideally, we would also test the other failures for other 'list' types ;-)

This review is powered by Dreditor.

but they only apply to the form that's generated by field_ui

Not exactly. The code is in a hook that is called by field_ui and won't be executed if field_ui isn't enabled, but this doesn't change the fact that it's code from list.module that is being tested.
Exact same reason why other list tests, text tests, options tests, etc. are in the field type modules, and not in field API.

do we need some kind of error handling or unit tests for the API functions to make sure API users don't make the same mistake?

You mean field_create_field() should throw an exception if the list of values is not valid ? Well, theoretically yes. Er, I'm not sure we'll be there in D7, though.

Great ! Minor remarks:

+++ modules/field/modules/list/list.test	23 Nov 2009 19:43:48 -0000
@@ -85,5 +85,96 @@ class ListFieldTestCase extends DrupalWe
+      'group' => 'Field UI',

group should be 'Field Types'

+++ modules/field/modules/list/list.test	23 Nov 2009 19:43:48 -0000
@@ -85,5 +85,96 @@ class ListFieldTestCase extends DrupalWe
+    //Test Number type.

Nitpick: "Number" field type is something else.
I'd suggest "Test 'List (number)' field type.
Same remark applies to "Test Text type below".

As a side note: those 3 different 'list' types are on their way out: #622614-3: Consider reducing the number of field types

+++ modules/field/modules/list/list.test	23 Nov 2009 19:43:48 -0000
@@ -85,5 +85,96 @@ class ListFieldTestCase extends DrupalWe
+    $valid_types = array('list',
+                         'list_boolean',
+                         'list_number',
+                         'list_text',
+                         );
+    if (!in_array($type, $valid_types)) return FALSE;

That check is overkill. Plz just remove it.

Really nitpick: please put the tests in the following order:
- List
- List (number)
- List (text)
- Boolean

Once those are fixed, this is RTBC.

This review is powered by Dreditor.

Oops, 2 last really minor details.

+++ modules/field/modules/list/list.test	23 Nov 2009 23:33:26 -0000
@@ -85,5 +85,89 @@ class ListFieldTestCase extends DrupalWe
+      'name' => 'List Field UI tests',

Don't capitalize the F(ield), to be consistent with the existing 'List field tests'

+++ modules/field/modules/list/list.test	23 Nov 2009 23:33:26 -0000
@@ -85,5 +85,89 @@ class ListFieldTestCase extends DrupalWe
+    // Test 'List (number)' field type.

The other comments don't have the word 'field'

This review is powered by Dreditor.

:-p

Thanks matt !

Looks great. Great tests too, matt. Committed to CVS HEAD.

Quickfix: test category should be 'Field Types', not 'Field types'. Now we have both on admin/config/development/testing (and it causes the JS expand/collapse to act strangely BTW).

+ trailing whitespace fixes.
hint to @matt2000 for later patches: would be great if you configured your IDE to remove them automatically on file save ;-)

Well, actually, core's capitalization standard is "Sentence case", so we should fix the one that are "Field Types" to "Field types" so they're all consistent.

re @webchick #24: OK - #658358: inconsistencies in field type tests 'Group'.
Setting this one back to 'fixed'.