This is a great module, but it sort of worries me that there are no tests written for it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

Feel free to start writing tests or to hire someone to do it.

bulat’s picture

I guess that should be done while functionality is added or bugs are fixed as well. We will write test for subscriptions reference module that we are developing. I will post our integration tests once they are ready.

bulat’s picture

Here is what we came up with so far:

http://drupalcode.org/sandbox/bulat/1990654.git/blob/HEAD:/subscriptions...

Following class can be used as a base for test cases for Subscriptions:

/**
 * Base test case class for Subscriptions
 */
class SubscriptionsTestCase extends DrupalWebTestCase {
  function setUp() {
    $modules = array_merge(array('subscriptions'), func_get_args());
    parent::setUp($modules);
  }
  
  /**
   * Gets records from subscriptions_queue table
   * @param array $conditions
   * @return array queue items - fetched records from queue table
   */
  protected function getSubscriptionQueueItems($conditions = array()) {
     $query = db_select('subscriptions_queue', 'sq')
      ->fields('sq', array('sqid', 'value', 'field', 'uid', 'load_args',));
     foreach ($conditions as $field => $value) {
       $query->condition($field, $value);
     }
     $queue = $query->execute()->fetchAll();
     return $queue;
  } 
  
  /**
   * Asserts number of subscription that exist for specificed parameter for user
   */
  protected function assertSubscriptions($params, $uid, $count, $message = '') {
    $s = subscriptions_get($params);
    if ($count == 0) {
      // check that user has no subscriptions
      $this->assertFalse(isset($s[$uid]), $message ? $message : t('User has no subscriptions.'));
    } 
    else {
      // check if that and only item is in the queue (uA is autosubscribed)
      $this->assert(isset($s[$uid]) && count($s[$uid]) == $count, $message ? $message : t('User has @count subscriptions.', array('@count' => $count)));
    }
  }
  
  /**
   * Asserts number of items in subscription queue for specified conditions
   */
  protected function assertSubscriptionsQueue($conditions, $items, $message ='') {
    $queue = $this->getSubscriptionQueueItems($conditions);
    if ($items == 0) {
      $this->assertFalse($queue, $message ? $message : t('Subscriptions queue is empty for provided conditions.'));
    }
    else {
      $this->assertEqual(count($queue), $items, $message ? $message : t('@items items found in subscriptions queue.', array('@items' => $items)));
    }
  }
  
}
salvis’s picture

This looks very interesting, but please post a patch. Reviewing code that is not in patch format is difficult and error-prone.

bulat’s picture

Status: Active » Needs review
FileSize
7.57 KB

Please see the patch with the base class SubscriptionsTestCase that extends DrupalWebTestCase and sample test for subscriptions_content.

Status: Needs review » Needs work

The last submitted patch, subscriptions-initial_test_cases_added-1984584-5.patch, failed testing.

bulat’s picture

Status: Needs work » Needs review
FileSize
8.42 KB

Added include to fix the test case.

salvis’s picture

Wow, this is great — thank you very much!

Unfortunately, you have quite a few violations of the http://drupal.org/coding-standards

+++ b/subscriptions.info
@@ -4,3 +4,6 @@ package = "Subscriptions"
+files[] = subscriptions_content.test
\ No newline at end of file

Needs trailing newline.

+++ b/subscriptions.test
@@ -0,0 +1,61 @@
+ * Contains base class for test cases for Subscriptions module

Comments must end with a period.

+++ b/subscriptions.test
@@ -0,0 +1,61 @@
+  ¶

The first of many trailing spaces.
Use Dreditor to look at your patch, or set your editor to remove those trailing spaces.

+++ b/subscriptions.test
@@ -0,0 +1,61 @@
+  /**
+   * Gets records from subscriptions_queue table
+   * @param array $conditions
+   * @return array queue items - fetched records from queue table
+   */

This must be formatted as (see http://drupal.org/node/1354):

+  /**
+   * Gets records from subscriptions_queue table.
+   *
+   * @param array $conditions
+   *   An array of conditions of the form ('field' => value).
+   *
+   * @return
+   *   An array of fetched records from queue table.
+   */

+++ b/subscriptions.test
@@ -0,0 +1,61 @@
+  protected function getSubscriptionQueueItems($conditions = array()) {
+     $query = db_select('subscriptions_queue', 'sq')
+      ->fields('sq', array('sqid', 'value', 'field', 'uid', 'load_args',));

Always indent 2 positions per level.

+++ b/subscriptions.test
@@ -0,0 +1,61 @@
+      // check that user has no subscriptions

All comments must start in upper case and end with a period.

+++ b/subscriptions_content.test
@@ -0,0 +1,130 @@
+class SubscriptionsContnentTestCase extends SubscriptionsTestCase {

Typo.

+++ b/subscriptions_content.test
@@ -0,0 +1,130 @@
+  /**
+   * Tests subscribe functions used by rules
+   * @see subscriptions_write_subscription()
+   * ¶
+   */

Period at the end of the first line, then an empty line; no trailing empty line.

These are all minor issues, but the sooner you learn to follow the Drupal guidelines the better you'll fit into the Drupal community.

Requiring third-party libraries (pecl, apd, runkit) is not supported by the testbot AFAIK, which would defeat the automated testing, unfortunately.

bulat’s picture

Thank you for the review. Here is the new patch.

vuil’s picture

Issue summary: View changes
Status: Needs review » Needs work

The patch #9 needs to be re-rolled.