Problem

No automated tests.

Proposal

Now that the functionality is starting to settle, add automated tests based on the user interface. The underlying API is already tested in core, so we need to test the UI. Test at least one non-entity based config, I think testing both the site info page and the user settings page would be great (since the user settings will be comprised of two .yml files). Plus testing one config entity like contact category or vocabulary (or if feeling ambitious something like a View would be great). But views don't have good schema support yet.

I think testing the config entity stuff should be postponed on further completion of the translation tabs issue, but tests for the site config editing page, that is pretty much at its final place (I believe). So starting with that as a first step would be great.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

Status: Active » Needs review
FileSize
24.61 KB
3.28 KB

Here is our first test to translation of site name and slogan in French...

Locally, they are looking good:

2013-03-26_010820.png

vijaycs85’s picture

FileSize
3.34 KB

Cleaning up....

Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: +sprint

Looking good in general, no major notes. :)

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+/**
+ * @file
+ * Definition of Drupal\config_translation\Tests\ConfigTranslationUITest.

This should currently be Contains \Drupal...

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+/**
+ * Functional tests for the language list configuration forms.
+ */

Language list? :)

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+    $this->drupalGet('admin/config/system/site-information');

Not needed since you are not actually asserting anything on this. Instead put the URL on the drupalPost.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+    $edit =array(

Space missing after =

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+    $this->drupalPost(NULL, $edit, t('Save configuration'));

I'd putt the URL here instead of NULL here, makes it easier to see what is going on and also easier to refactor tests up above it, so we don't rely on the page loaded earlier (which is pointless ATM anyway).

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+    $translatable = $this->assertLinkByHref($translation_base_url);
+    if ($translatable) {

We usually let these tests fail after it, instead of sealing them off if the earlier one failed. Especially in this case, the first one will be a very prominent fail. So we usually don't make these conditional.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+      $this->drupalPost(NULL, $edit, t('Save translation'));

How does this work? You are not actually testing the add/fr page?! The last drupalGet() got the translation base URL. Again best to include the concrete URL for ease of maintenance.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+      // Check translation saved proper.
+      $this->drupalGet($translation_base_url . '/edit/fr');
+      $this->assertFieldByName('0[name]', 'FR ' . $site_name);
+      $this->assertFieldByName('0[slogan]', 'FR ' . $site_slogan);

This is good, however, I'd also check /fr on the site to see if the site name and slogan displayed. Maybe only the site name is displayed by default. You can hit to configure the theme settings to make the slogan appear as well.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
23.43 KB
3.46 KB
4.31 KB

Thanks @Gábor Hojtsy. Fixed all review comments in #3.

Reg:

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -0,0 +1,98 @@
+      $this->drupalPost(NULL, $edit, t('Save translation'));

How does this work? You are not actually testing the add/fr page?! The last drupalGet() got the translation base URL. Again best to include the concrete URL for ease of maintenance.

We got 'click' to visit the page.

   // Check 'Add' link of French to visit add page.
    $this->assertLinkByHref($translation_base_url . '/add/fr');
    $this->clickLink(t('Add'));

however I added URL in post.

Maybe only the site name is displayed by default. You can hit to configure the theme settings to make the slogan appear as well.

Locally it is showing both...
2013-03-26_195442.png

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, committed. We should keep expanding on this in various ways, but this should be a great start. Also allows me to turn on automated testing for the project so we get testbot results for patches :)

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

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