Moved over from #1224838-14: Fix the inconsistency between filenames as array keys and array values.
We can do better than what we currently have.

CommentFileSizeAuthor
#14 1299076-14-libraries-better-tests.patch3 KBtstoeckler
#11 1299076-11-libraries-better-tests.patch3 KBtstoeckler
#9 1299076-9-libraries-better-tests.patch2.99 KBtstoeckler
#7 1299076-7-libraries-better-tests.patch2.93 KBtstoeckler
#5 1224838-libraries-better-tests-2-debug.patch3.04 KBtstoeckler
1224838-libraries-better-tests-2.patch3 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

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

Status: Needs review » Needs work

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

I don't get what I'm doing wrong. I get all greens with this one...

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

You should be able to see "live" values on the testbot by squeezing in debug() lines.

btw, let's also rename $found to $expected.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
1224838-libraries-better-tests-2-debug.patch3.04 KB

Thanks for the tip, I wasn't aware of that!
Let's try this one.

Status: Needs review » Needs work

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
1299076-7-libraries-better-tests.patch2.93 KB

Damn you, unclean URLs!!!
Either way, this revealed a little bit of cruft which can be removed. Let's see if this one works.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Needs work 
+++ b/tests/libraries.test
@@ -410,24 +410,40 @@ class LibrariesTestCase extends DrupalWebTestCase {
+        $message = ($expected ? "$name.$extension found." : "$name.$extension not found.");
...
+        ($expected ? $this->assertRaw($raw, $message) : $this->assertNoRaw($raw, $message));

Especially the latter line is a coding style not used in Drupal. Looks like a simple if/else control structure would be more appropriate here.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
1299076-9-libraries-better-tests.patch2.99 KB

How about this one?

Status: Needs review » Needs work

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
1299076-11-libraries-better-tests.patch3 KB

How on earth did that pass locally?!!?!?
Let's try this one.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Needs work 
+++ b/tests/libraries.test
@@ -410,24 +410,44 @@ class LibrariesTestCase extends DrupalWebTestCase {
+    foreach ($names as $name => $expected) {
       foreach ($extensions as $extension) {
...
+        $label = ($label !== '' ? "$label: " : '');

We either need a different assignment variable here or we need to move that label munging simply out of the foreachs.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Sorry, I thought I had checked the messages, but evidently I didn't:

Version overloading: example_1.js found.
Version overloading: : example_1.css found.
Version overloading: : : example_1.php found.
Version overloading: : : : example_2.js found.
Version overloading: : : : : example_2.css found.
Version overloading: : : : : : example_2.php found.
Version overloading: : : : : : : example_3.js found.
Version overloading: : : : : : : : example_3.css found.
Version overloading: : : : : : : : : example_3.php found.
Version overloading: : : : : : : : : : example_4.js found.
Version overloading: : : : : : : : : : : example_4.css found.
Version overloading: : : : : : : : : : : : example_4.php found.

Will reroll with $label moved out of the foreach.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
1299076-14-libraries-better-tests.patch3 KB

There we go.

tstoeckler’s picture

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

Gave this another look, and decided to commit this.
http://drupal.org/commitlog/commit/10030/abd2625f62f346613c7aedd74ad067e...

We already introduced one regression because of our poor current tests. This works and is definitely an improvement of the status quo. We can always refine in follow-ups. And at some point we're going to have to overhaul our test case anyway...

Status: Fixed » Closed (fixed)

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