search

matthiasmullie / scrapbook

PHP cache library, with adapters for e.g. Memcached, Redis, Couchbase, APC(u), SQL and additional capabilities (e.g. transactions, stampede protection) built on top.
https://www.scrapbook.cash
MIT License
315 stars 27 forks source link

Added integration tests #6

Closed Nyholm closed 8 years ago

Nyholm commented 8 years ago

I also updated the PHP version. We do not test with php53 so we dont know if it works or not. Also php53 has not been officially supported since 2014.

matthiasmullie commented 8 years ago

I'd prefer to keep PHP5.3 support around for wherever it still makes sense (= unless it lacks specific features I need). So far, except for the Flysystem adapter (which uses league/flysystem, which is PHP5.4+), everything still supports 5.3.

I know it's been unsupported for awhile now, but it came by default with Ubuntu 12.04, which is still supported (LTS) and happens to be one of the major distros in use. So while I don't necessarily actively want to keep supporting 5.3, I'm also not actively going to break compatibility with 5.3 unless there's a really good reason :)

Anyway, I side-stepped there for a minute. I had also seen that part in your other PR but didn't yet have the time to dig into this. The other existing tests can also run on all adapters, whereas the way this one is built it can only run with 1 adapter. I haven't really been able to think much about how (if at all) I could also just let it run for every adapter. IDK yet. I'll look into that later, but I definitely also want to include this test suite as well. I just haven't had the time to properly look into this, so far. I'll definitely get back to this!

Nyholm commented 8 years ago

Sure, I've have different opinion about php53. I reverted my change to support php53.

The other existing tests can also run on all adapters, whereas the way this one is built it can only run with 1 adapter. I haven't really been able to think much about how (if at all) I could also just let it run for every adapter. IDK yet. I'll look into that later, but I definitely also want to include this test suite as well. I just haven't had the time to properly look into this, so far. I'll definitely get back to this!

It does. In your travis config you filter the test on the ADAPTER environment variable. Which means you only run one adapter per travis test case.

In my test class I read from the ADAPTER to know what to instantiate. So I do also run one adapter per travis test case.

matthiasmullie commented 8 years ago

The existing tests allowed for tests to be run on just 1 specific adapter, but also against all adapters. It did that through a @dataProvider that just provided all adapters. But I couldn't just go change your test suite to add @dataProviders in there, because it wouldn't make sense in your context.

So I didn't really know how to also be able to run the tests against all adapters - that's why it took a while for me to merge this code in.

I figured out a way, though. This is what I've changed the test to: https://github.com/matthiasmullie/scrapbook/blob/master/tests/Psr6/Taggable/TaggableCacheTest.php (it uses https://github.com/matthiasmullie/scrapbook/blob/master/tests/AdapterProvider.php)

Everything's now also running your test suite - thanks!

Nyholm commented 8 years ago

Cool. Good job!