Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor query caching tests #5008

Merged
merged 1 commit into from
Nov 21, 2021
Merged

Refactor query caching tests #5008

merged 1 commit into from
Nov 21, 2021

Conversation

morozov
Copy link
Member

@morozov morozov commented Nov 21, 2021

Q A
Type improvement
BC Break no

The primary goal of this patch is to get rid of the dependency of the test suite on the DebugStack class which will be later deprecated in #4967.

Additionally, the current CachedQueryTest has the following issues:

  1. There isn't much point in testing a caching layer via functional tests. The caching layer by its nature shouldn't affect the functionality of the component that it wraps. Only the interaction between the components needs to be tested.
  2. Most of the tests cover fetching data from the cache in different modes and their combinations. Those tests are irrelevant as of Always cache the full result #5003.
  3. Testing different cache implementations makes sense but testing that replacing a cache implementation invalidates the cache is enough to be tested just with the default implementation because this logic doesn't depend on the implementation.

@morozov morozov added this to the 3.2.0 milestone Nov 21, 2021
@morozov morozov requested review from derrabus and greg0ire November 21, 2021 00:56
@derrabus derrabus merged commit 635376e into doctrine:3.2.x Nov 21, 2021
@morozov morozov mentioned this pull request Nov 21, 2021
3 tasks
@morozov morozov deleted the cache-tests branch January 1, 2022 00:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants