Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comments
Comment #1
ayang CreditAttribution: ayang commentedComment #3
ayang CreditAttribution: ayang commentedGit patch.
Comment #4
ayang CreditAttribution: ayang commentedDrupal 6 backport.
Comment #5
coltraneUsers 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?
Comment #6
ayang CreditAttribution: ayang commentedPer 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.
Comment #7
ayang CreditAttribution: ayang commentedAdded the drush command and merged everything into Acquia SPI.
Comment #9
ayang CreditAttribution: ayang commented#7: 2088603-acquia_connector-user_contrib_tests_d7-7.patch queued for re-testing.
Comment #11
ayang CreditAttribution: ayang commentedSwitching review to D7.
Comment #12
coltraneLooking good and functional! Just some minor updates I think.
How about spi-test-validate?
Could also recommend the drush command here. You could add a help link here and some text like the following to acquia_agent.module:
"from custom test callbacks"
This hook supports multiple tests so long as they each have a unique identifier right? Perhaps add a comment saying so.
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.
Comment #13
ayang CreditAttribution: ayang commentedAdded these updates:
Comment #14
coltraneI 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.
Comment #15
coltraneCommitted http://drupalcode.org/project/acquia_connector.git/commit/ba663c8 and http://drupalcode.org/project/acquia_connector.git/commit/691e22a
Comment #16
ayang CreditAttribution: ayang commented:D