Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements Tests for AddThis #51

Merged
merged 5 commits into from
Sep 4, 2015
Merged

Conversation

doylejd
Copy link
Contributor

@doylejd doylejd commented Aug 27, 2015

Updates

  • Adds unit tests for Block, Field and Permissions.
  • Resolves several issues found from going through testing
  • Does a bit of code cleanup

TODO

  • Need to do some abstraction in the tests. There is a lot of duplication that could be cleaned up.
  • More tests are needed to get full code coverage.
  • Should be able to verify markup on the page, but need to confirm.

@@ -56,7 +56,8 @@ public function getServices() {
'#type' => 'inline_template',
'#template' => '<span class="addthis_service_icon icon_' . $serviceCode . '"></span> ' . $serviceName,
);
$rows[$serviceCode] = $service;
//#options expects a string, not an array. Render the element so it becomes a string.
$rows[$serviceCode] = render($service);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, Controller? It's rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be moved as part of another issue. Had to call render() here because of the nature of the nested form options to avoid getting 500 warnings on every load of the admin page.

nerdstein added a commit that referenced this pull request Sep 4, 2015
Implements Tests for AddThis
@nerdstein nerdstein merged commit 21a2ee0 into d8-contrib-modules:master Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants