search

kirkbushell / eloquence

A drop-in library for certain database functionality in Laravel, that allows for extra features that may never make it into the main project.
MIT License
537 stars 58 forks source link

Sum cache #35

Closed cviebrock closed 8 years ago

cviebrock commented 8 years ago

Implements the SumCache idea mentioned in #34. :smile:

cviebrock commented 8 years ago

So, I'm not sure why the tests are failing. If I run each set of tests individually, they both pass:

./vendor/bin/phpunit ./tests/Acceptance
./vendor/bin/phpunit ./tests/Unit

But running the whole suite fails:

1) Tests\Acceptance\CountCacheTest::testUserCountCache
Mockery\Exception\InvalidCountException: Method update("UPDATE `orders` SET `item_total` = `item_total` + (34) WHERE `id` = 2") from Mockery_0 should be called
 exactly 1 times but called 0 times.

Which is odd, because that test and method doesn't match up with the exception.

Any ideas?

kirkbushell commented 8 years ago

Thanks for this work. I feel however that some of the language isn't quite right, due to the fact that it was copied from count cache. For example, I do not think increment/decrement works in this context. Perhaps increase/decrease, or something else?

kirkbushell commented 8 years ago

As for the tests, I'll check them out later today. Feeling much better but I have work to do :)

cviebrock commented 8 years ago

Fair enough ... I will review the language and see if I can make it more applicable. I'll also adjust the PR to accept the "explicit" style configuration from my other PR.

kirkbushell commented 8 years ago

Cool okay, np.

cviebrock commented 8 years ago

Renamed some methods, updated tests, and added new tests for explicit config syntax.

Full test suite still fails, but they pass if you run the Unit and Acceptance tests separately.

I leave it to you now. ;)

kirkbushell commented 8 years ago

I get a different problem, as it's not setting up the connection properly.

kirkbushell commented 8 years ago

I've fixed up these tests and merged in the resulting fixes. Pretty cool github actually recognised that I pulled down your branch locally and had worked on it then pushed it here. Nice :)

There's still a bit of testing to be done I feel regarding this work and a few other PRs that have come in recently. Plus I'm addressing some issues I have with the API, in addition to improving the usability of the library, so a 1.5 release is still a ways off.

cviebrock commented 8 years ago

No problem, thanks! This is going to be super-helpful for my current project ... which is usually why I make PRs. ;)