libraries.test really is one ugly piece of ...
... annoying PHP code.

This is a first step, there's definitely lots more to do.
But this shouldn't hurt. Depending on how motivated I am to do more clean-up, I might commit this in a few days as is. Don't know yet.

CommentFileSizeAuthor
#10 libraries-slightly-better-tests-4.patch6.73 KBtstoeckler
#7 libraries-slightly-better-tests-3.patch6.67 KBtstoeckler
#2 libraries-slightly-better-tests-2.patch3.34 KBtstoeckler
libraries-slightly-better-tests.patch3.27 KBtstoeckler

Comments

Status: Needs review » Needs work

The last submitted patch, libraries-slightly-better-tests.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new libraries-slightly-better-tests-2.patch3.34 KB

Ahh, I'm an idiot.

Status: Needs review » Needs work

The last submitted patch, libraries-slightly-better-tests-2.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ b/tests/libraries.test
@@ -6,33 +6,33 @@
-  function setUp() {

We will still need setUp() to load the libraries.module that contains the functions we want to call.

Tests can be run without installing the corresponding/originating module.

+++ b/tests/libraries.test
@@ -6,33 +6,33 @@
+  testLibrariesGetPath() {
...
+  testLibrariesPrepareFiles() {

@@ -49,8 +49,12 @@ class LibrariesTestCase extends DrupalWebTestCase {
+  testLibrariesDetectDependencies() {

Missing "function " prefix here ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ b/tests/libraries.test
@@ -6,33 +6,33 @@
+  public static function testLibrariesGetPath() {
...
+  public static function testLibrariesPrepareFiles() {

@@ -49,8 +49,12 @@ class LibrariesTestCase extends DrupalWebTestCase {
+  public static function testLibrariesDetectDependencies() {

test methods are not static :)

As of now, we also don't specify any visibility for test methods, so just make that "function testFoo()"

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review

oh God, I knew it was late, but I didn't think it'd be that late. :)
I guess I'll stop for now. This is getting worse and worse :)
Thanks for the review though.
I'll take this for a spin in the next few days.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
StatusFileSize
new libraries-slightly-better-tests-3.patch6.67 KB

Here we go.

Status: Needs review » Needs work

The last submitted patch, libraries-slightly-better-tests-3.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ b/tests/libraries.test
@@ -6,33 +6,33 @@
+class LibrariesUnitTestCase extends DrupalUnitTestCase {
...
-  function setUp() {

The unit test needs to manually load Libraries module, because it may not be enabled in the parent site that runs the test - hence, nothing in libraries.module is loaded.

I.e.:

function setUp() {
  drupal_load('module', 'libraries');
}
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new libraries-slightly-better-tests-4.patch6.73 KB

Ahh, I didn't know that. Thanks!

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. Hopefully the testbot will confirm. :)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Reviewed & tested by the community » Fixed

Thanks again for the reviews.
Committed this to 7.x-2.x
http://drupal.org/commitlog/commit/10030/8f8f9b4594a4a001171834f193b242c...

Status: Fixed » Closed (fixed)

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