Improve consistency of tests used on L1s.#71
Conversation
composer.json
Outdated
| "autoload":{ | ||
| "psr-4":{ | ||
| "LCache\\": "src" | ||
| "LCache\\": ["src/", "tests/"] |
There was a problem hiding this comment.
Does this need to go in autoload, or could you put it in autoload-dev? I prefer to put autoloaded classes used by the tests in tests/src, to differentiate them from files that are loaded by phpunit because they are test files. See the existing entry in autoload-dev. No one is using the LCache\TestUtils namespace; I just put it in as an example. This could just be LCache. You could make the directory tests instead of tests/src too. But I'd recommend keeping it out of autoload.
There was a problem hiding this comment.
Thanks @greg-1-anderson! You're right and I'll update this file.
tests/LCacheTest.php
Outdated
|
|
||
| public function testEmptyCleanUpDatabaseL2() | ||
| { | ||
| // Why does this test make no assertions? |
There was a problem hiding this comment.
I believe it's testing that destructors run properly (without errors/exceptions) with an empty data set. Because all other tests write data, they don't trigger the destuctor in an empty state.
|
I just had a verbal conversation with Greg and David. They consider this PR ready to merge and would like #72 broken apart so that L1s and L2s can be addressed in separate PRs. @davidstrauss Can you now "Squash and merge" this PR? I don't know why Coveralls hasn't reported back. The most recent commit was just a comment removal. |
Overview
This pull request:
Summary
This PR is the first step in #70 and is a precursor to a test refactoring in #72
Description
The test suite currently uses a convention of helper methods (
performL1HitMissTest(),performL1AntirollbackTest()etc) along withtest*methods per implementation (testStaticL1Antirollback(),testSQLiteL1Antirollback()). I would like to switch to using inheritance or traits to share these tests among implementations (#72). Before considering that pull request, I'd like to first validate that all of these tests are appropriate to run on all L1s (with the exception of NullL1).This pull request also makes a change to resolve inconsistent handling of hit/miss counting between the L1 implementations. Right now only the
APCuL1resets its hit/miss count as a result of a cache clear. This PR makes it so thatStaticL1andSQLiteL1also reset their counts as the result of a cache clear.The PR also fixes a bug in
APCuL1::setWithExpiration()where the$createdvariable was not used.Todo
testEmptyCleanUpDatabaseL2()makes no assertions. (I suppose this is not a hard blocker but I'd like to get this answered before refactoring tests further)