Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I saw you wanted to test this in #1186636: Provide a 2.x alpha release of Libraries API, and I had this old patch lying around in #1122224: Cannot load AmazonS3 class. Please check the awssdk is installed correctly, so let's see if that still applies.
Comment | File | Size | Author |
---|---|---|---|
#12 | 1205854-12-libraries-test-cache.patch | 4.41 KB | tstoeckler |
#9 | 1205854-libraries-test-cache.patch | 4.23 KB | tstoeckler |
#2 | 1205854-2-libraries-caching.patch | 968 bytes | tstoeckler |
1122224-test-libraries-caching-2.patch | 1.01 KB | tstoeckler | |
Comments
Comment #2
tstoecklerLet's try this one.
Comment #3
sunHm, not sure whether this test makes sense. The following would make more sense to me, but I'm not sure whether it is possible to test, and whether it's worth the effort:
Otherwise, we're merely testing core's Cache API and not really our own caching mechanism.
But I somewhat doubt that these (pseudo) assertCacheMiss() and assertCacheHit() assertions are possible without introducing a really large chunk of testing code that might be a hack even.
Though, perhaps set a variable and just simply conditionally output a message in hook_libraries_info() of the testing module -- which should then only be triggered on a cache miss, if I get this right?
Comment #4
tstoecklerIf we do a drupal_set_message() libraries_test_libraries_info() that will effect all other tests. Otherwise we would have to make a new libraries_cache_test.module, just for that purpose.
What would also work, and be simpler IMO, is putting a drupal_set_message() in one of the callbacks (except the 'load' group, which gets called every time) and then assert on that message.
That should be doable, I think.
Comment #5
sunPutting it into one of the cached callbacks sounds good. By wrapping it into a condition
if (variable_get('libraries_test_cache', FALSE)
, it will only be output and affect this test.Comment #6
tstoecklerAhh, that's what you meant with the variable. Yes, that absolutely makes sense.
Comment #7
sunComment #8
tstoecklerNeeds work.
(And bumping. (For myself to see this tomorrow or so and pick it up.))
Comment #9
tstoecklerHere we go. I did a minimal cleanup in libraries_test_menu(), hope that doesn't put you off.
The tests pass, so the caching does work as intended.
Comment #10
tstoecklerJust looked at this again, and couldn't find anything wrong with it. Since it's just tests for existing functionality, I'll probably commit this soon-ish.
Comment #11
sunLet's define
$base = array(...);
and + that into the others:
$items[...] = $base + array(...);
Aside from that, this looks good to me.
Comment #12
tstoecklerSure thing.
If this comes back green, I'll commit it.
Comment #13
tstoecklerCommitted to 7.x-2.x.
Thanks for the quick feedback!
http://drupal.org/commitlog/commit/10030/e2472883c6dce4c75b66f41c08fc1db...