Extend the current Acquia SPI module to handle user-contributed tests. Also provide some data validation to ensure that the data interacts with Insight properly along with error feedback for data which fails validation.

Files: 
CommentFileSizeAuthor
#14 2088603-acquia_connector-user_contrib_tests_d6-14-do-not-test.patch12.61 KBcoltrane
#14 2088603-acquia_connector-user_contrib_tests_d7-14.patch16.07 KBcoltrane
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]
#13 2088603-acquia_connector-user_contrib_tests_d6-13.patch15.5 KBayang
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d6-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 2088603-acquia_connector-user_contrib_tests_d7-13.patch16.04 KBayang
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]
#11 2088603-acquia_connector-user_contrib_tests_d7-10.patch12.47 KBayang
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]
#7 2088603-acquia_connector-user_contrib_tests_d6-7.patch12.33 KBayang
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#7 2088603-acquia_connector-user_contrib_tests_d7-7.patch12.47 KBayang
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d7-7_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#4 2088603-acquia_connector-user_contrib_tests_d6-4.patch18.38 KBayang
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#3 2088603-acquia_connector-user_contrib_tests_d7-3.patch16.84 KBayang
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d7-3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 2088603-acquia_connector-user_contrib_tests.patch17.92 KBayang
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

StatusFileSize
new17.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Status:Needs review» Needs work

The last submitted patch, 2088603-acquia_connector-user_contrib_tests.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new16.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d7-3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Git patch.

Version:7.x-2.x-dev» 6.x-2.x-dev
StatusFileSize
new18.38 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Drupal 6 backport.

Status:Needs review» Needs work

Users are a bit confused about the Acquia Connector being multiple modules and there's a JIRA issue to eventually combine Agent and SPI into one module. For that reason I don't think custom tests should be a separate module. Would you be OK with moving this all to Acquia SPI? acquia_spi_test_collect() could become part of acquia_spi.module just fine.

I'm not convinced of the usefulness of storing test failures and providing a page callback. I understand developers would like to validate their tests but I would like to see if there's a lighter weight solution. Perhaps a "validate custom tests" link on the admin reports status page that either watchdog()s or prints the results in a drupal_set_message()? Or perhaps a drush command for test validation? I think authors of custom tests would be comfortable with drush. What do you think?

Per our discussion earlier, that shouldn't be a problem. I'll move the relevant functions over into the Acquia SPI module. I can go either way on the best way to log these, but I don't mind going for the lighter weight option. I'll also add a drush command the next iteration.

Status:Needs work» Needs review
StatusFileSize
new12.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d7-7_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new12.33 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Added the drush command and merged everything into Acquia SPI.

Status:Needs review» Needs work

The last submitted patch, 2088603-acquia_connector-user_contrib_tests_d7-7.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 2088603-acquia_connector-user_contrib_tests_d7-7.patch, failed testing.

Version:6.x-2.x-dev» 7.x-2.x-dev
Status:Needs work» Needs review
StatusFileSize
new12.47 KB
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]

Switching review to D7.

Looking good and functional! Just some minor updates I think.

  1. +++ b/acquia_spi/acquia_spi.drush.inc
    @@ -19,6 +20,11 @@ function acquia_spi_drush_command() {
    +  $items['spi-validate'] = array(

    How about spi-test-validate?

  2. +++ b/acquia_spi/acquia_spi.install
    @@ -75,6 +75,32 @@ function acquia_spi_requirements($phase) {
    +          $description = 'Custom tests within the following module(s) have failed validation and will not be sent: %modules. <br/>Please check your error logs for more information regarding how to pass validation.';

    Could also recommend the drush command here. You could add a help link here and some text like the following to acquia_agent.module:

    diff --git a/acquia_agent/acquia_agent.module b/acquia_agent/acquia_agent.module
    index eb865af..f9b4d0a 100644
    --- a/acquia_agent/acquia_agent.module
    +++ b/acquia_agent/acquia_agent.module
    @@ -551,6 +551,8 @@ function acquia_agent_help($path, $arg) {
           $output .= '<dd>' . t('Receive dynamic updates on the Network Settings page from Acquia.com about your subscription and new features.') . '</dd>';
           $output .= '<dt>' . t('Allow Insight to update list of approved variables.') . '</dt>';
           $output .= '<dd>' . t('As part of the Acquia Insight service, some variables can be corrected to their recommended settings from within the Insight system. The list of variables that can be corrected can also be updated at your discretion.') . '</dd>';
    +      $output .= '<dt>' . t('Acquia SPI custom tests.') . '</dt>';
    +      $output .= '<dd>' . t('Acquia Insight supports custom tests for your site. See acquia_spi.api.php for information on the custom test hook and validate your tests for inclusion in outgoing SPI data with the Drush command "spi-test-validate".') . '</dd>';
           $output .= '</dl>';
           return $output;
       }
  3. +++ b/acquia_spi/acquia_spi.module
    @@ -598,6 +605,164 @@ function _acquia_spi_security_review_compatible() {
    + * Validates data from callbacks.

    "from custom test callbacks"

  4. +++ b/acquia_spi/acquia_spi.api.php
    @@ -33,3 +33,33 @@ function hook_acquia_spi_get() {
    + * careful not to send sensitive information in any field.

    This hook supports multiple tests so long as they each have a unique identifier right? Perhaps add a comment saying so.

  5. +++ b/acquia_spi/acquia_spi.module
    @@ -598,6 +605,164 @@ function _acquia_spi_security_review_compatible() {
    +

    For string attributes, how about also limiting the length? That way someone can't send more than X characters in a field (unless they hack it, but then we don't have to support it). Maybe a limit of 1024 characters? Please also add to the hook documentation.

StatusFileSize
new16.04 KB
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]
new15.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2088603-acquia_connector-user_contrib_tests_d6-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Added these updates:

  1. Makes sense to make it a bit more specific. Changed it.
  2. Added the help link and a menu callback to perform a validation check. Added a help section to Acquia Agent, and also added "more information" links onto the status page.
  3. Whoops! Added.
  4. Cleaned up the acquia_spi.api.php a bit and added notes about the unique identifiers.
  5. Added the string character cap and notes for it.

StatusFileSize
new16.07 KB
PASSED: [[SimpleTest]]: [MySQL] 406 pass(es).
[ View ]
new12.61 KB

I wasn't clear in chat about the access arguments, sorry, thought there was a permission on that callback. I modified the test validate callback to use permission 'access site reports'. Also added note about character count to error message.

This looks good, thanks so much for your work on this @ayang!!

I'll be committing these two versions after testbot shows d7 passing.

:D

Status:Fixed» Closed (fixed)

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