Taxonomy test cleanup

This will need a reroll.

wow. I hope drupal also has change notices of such strings changes. only a DOT.. if guys working on LDO, they should re-translate all these string again...

@droplet re: #3: This issue is not tagged for backport. Also, I'm fairly certain no one translates test assertions. In fact, there's an issue somewhere recommending people don't even use t() on them. It's just a cleanup...

Re-rolled. This patch applies cleanly at commit d8a4854.

Thanks @kid_icarus!

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -259,12 +259,12 @@ class TaxonomyVocabularyUnitTest extends TaxonomyWebTestCase {
+    $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {taxonomy_term_data}')->fetchField(), t('Terms deletion when vocabulary is removed.'));

The assertion message here is a little goofy. Let's rephrase this as:
Terms are deleted when the vocabulary is deleted.

Also, while we're cleaning these lines up, let's remove t() from the assertion message texts in the lines we are already changing. Reference:
http://drupal.org/simpletest-tutorial-drupal7#t

Thanks!

Update #5 with trying to fix #6 ;)

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -722,13 +726,13 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
+    $this->assertFalse(field_info_field($field_name), t('Field %field_name does not exist.', array('%field_name' => $field_name)));

This should use format_string(), not t()

Otherwise I think this is ready to go!

Addresses #8.

Looks great!

I think looks better ;)

Can you post an interdiff? I read through the patch in #9 very closely, I'd rather just see the changes made.

RTBC seconded!

I missed this in the queue, and just committed taxonomy entities as classed objects which very likely breaks this. Sending for re-testing.

I have reroll patch ;)

The following formatting changes are incorrect:

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -36,7 +37,7 @@ class TaxonomyWebTestCase extends DrupalWebTestCase {
-    ));
+      ));

@@ -52,15 +53,16 @@ class TaxonomyWebTestCase extends DrupalWebTestCase {
-    ));
+      ));

@@ -980,17 +985,17 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
-    ));
+      ));

@@ -1205,8 +1212,8 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1216,13 +1223,13 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();
...
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1232,13 +1239,13 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1257,13 +1264,13 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();
...
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1274,13 +1281,13 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();
...
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1291,13 +1298,13 @@ class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
-    ))->fetchField();
...
+      ))->fetchField();
...
-    ))->fetchField();
...
+      ))->fetchField();

@@ -1498,10 +1510,9 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
-    }
-    catch (FieldValidationException $e) {
...
+    } catch (FieldValidationException $e) {

@@ -1509,10 +1520,9 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
-    }
-    catch (FieldValidationException $e) {
...
+    } catch (FieldValidationException $e) {

So the current task is to reroll this patch without the above changes. Thanks!

Rerolled, addressing #21.

#22 looks good. Thanks @kid_icarus.

Attached just corrects the re-indented docblocks to use standard summaries and adds periods to a few of the (already updated) assertion messages that were missed.

This is RTBC. (I only made cosmetic changes.)

Also, despite my comment in #4... (I've learned a lot in the past four months.) This is an appropriate (and desirable) backport. None of these strings are actually translated in practice in D7, and it's better to keep the two codebases in line where possible.

Note that this issue should probably wait on thresholds and so might need a reroll again at some point before it goes in.

#23: taxonomy-cleanup-1195254-23.patch queued for re-testing.

Thanks folks. Committed/pushed to 8.x. It'd be good to get rid of the UnitTest that's not a unit test in there as well, but doesn't need to be done here.

Moving to 7.x for backport, I agree it's better to keep the code bases as in sync as possible - makes future backports easier.

Rerolled.

The diffstat is off because there was a whole hunk that was indented wrong in D8 that is still correct in D7.

Didn't I RTBC this backport already? Durr.

This looks good, but i'd like to hold it until after wednesday's release so we don't inadvertently break any other patches.

#27: drupal-1195254-27.patch queued for re-testing.

It's not going to apply anymore anyway :)

patch re-rolled

trying the re-roll again

oops. syntax error in #34 is fixed in this one

It appears to me that the patch works but the test for duplicate machine name failed.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -76,27 +77,27 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
     $this->drupalPost(NULL, $edit, t('Save'));
-    $this->assertRaw(t('Created new vocabulary %name.', array('%name' => $edit['name'])), t('Vocabulary created successfully'));
+    $this->assertRaw(format_text('Created new vocabulary %name.', array('%name' => $edit['name'])), 'Vocabulary created successfully.');

There is no format_text() in Drupal 7....

@trrroy: The patch in #27 is using t(), is it possible that you re-rolled the wrong one?

That was a typo for 'format_string'. Too much blood in my coffeestream. Let's try this again.

Shouldn't be format_string() either IMHO. Only test assertion messages should stop using t(), it still needs to be used for the actual strings that we are comparing against.

The patch in #27 was all good except the conflict because another issue was commited.

If it had been t() in the assertion message, it would be replaced with format_string(). But this is in the assertion itself, and should stay as t().

Yes, you're right. Thank for setting me straight, Berdir. Wow, messy noob attempt at reroll. I have a good feeling about this one.

The following all need to have t() restored to the first part of the assertion. It should only be removed from the message.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -76,27 +77,27 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
-    $this->assertText(t('The machine-readable name is already in use. It must be unique.'));
+    $this->assertText('The machine-readable name is already in use. It must be unique.');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -76,27 +77,27 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
-    $this->assertText(t('The machine-readable name must contain only lowercase letters, numbers, and underscores.'));
+    $this->assertText('The machine-readable name must contain only lowercase letters, numbers, and underscores.');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -153,29 +154,29 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
-    $this->assertText(t('Created new vocabulary'), t('New vocabulary was created.'));
+    $this->assertText('Created new vocabulary', 'New vocabulary was created.');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -153,29 +154,29 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
+    $this->assertRaw(format_string('Are you sure you want to delete the vocabulary %name?', array('%name' => $vocabulary->name)), '[confirm deletion] Asks for confirmation.');
+    $this->assertText('Deleting a vocabulary will delete all the terms in it. This action cannot be undone.', '[confirm deletion] Inform that all terms will be deleted.');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -153,29 +154,29 @@ class TaxonomyVocabularyFunctionalTest extends TaxonomyWebTestCase {
-    $this->assertRaw(t('Deleted vocabulary %name.', array('%name' => $vocabulary->name)), t('Vocabulary deleted'));
+    $this->assertRaw(format_string('Deleted vocabulary %name.', array('%name' => $vocabulary->name)), 'Vocabulary deleted');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -652,19 +655,19 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
-    $this->assertRaw(t('@type %title has been created.', array('@type' => t('Basic page'), '%title' => $edit["title"])), t('The node was created successfully'));
+    $this->assertRaw(format_string('@type %title has been created.', array('@type' => t('Basic page'), '%title' => $edit["title"])), 'The node was created successfully');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -684,29 +687,29 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
-    $message = t("Taxonomy field @field_name not found.", array('@field_name' => $field_name));
+    $message = format_string("Taxonomy field @field_name not found.", array('@field_name' => $field_name));

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -1510,7 +1513,7 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
-    $this->assertRaw(t('test_entity @id has been created.', array('@id' => $id)), t('Entity was created'));
+    $this->assertRaw(format_string('test_entity @id has been created.', array('@id' => $id)), 'Entity was created.');

.

+++ b/modules/taxonomy/taxonomy.testundefined
@@ -1645,7 +1648,7 @@ class TaxonomyTermFieldMultipleVocabularyTestCase extends TaxonomyWebTestCase {
-    $this->assertRaw(t('test_entity @id has been created.', array('@id' => $id)), 'Entity was created.');
+    $this->assertRaw(format_string('test_entity @id has been created.', array('@id' => $id)), 'Entity was created.');

.

I think patch #44 fixed all those issues in 45.

My #45 was a crosspost, #44 is RTBC.

Er. The original patch at the beginning of this issue was to add periods to the assertion messages that were missing them, among other things. The periods keep disappearing in the rerolls.

Right. Got those.

Looked over this again and compared it with the patch from #27. Looks good to me.

#50: drupal-1195254-50.patch queued for re-testing.

#50: drupal-1195254-50.patch queued for re-testing.

(Patch looks good; just want to make sure it still passes tests before committing.)

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/399cf0a

But actually, can we please fix this indentation bug (which the patch introduced) because it's driving me crazy :)

I think it's OK to RTBC my own D8 patch here.

Committed and pushed #56 to 8.x. :)

I'm going to count that as an RTBC for Drupal 7 also :)

Committed the followup there too. http://drupalcode.org/project/drupal.git/commit/f7d5d80