Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

fixes #6435: Race Condition in filesystem cache on prepare dir structure #6573

Closed
wants to merge 2 commits into from
Closed

Conversation

marc-mabe
Copy link
Member

This should fix the issue #6435
(unit testable only with mocking/wrapping filesystem)

@Ocramius we should have a discussion about fs mocking ;)

@Ocramius
Copy link
Member

@marc-mabe I'm fairly sure that you know more about the locking issues on the FS than me. What I'm worrying about is that we'd lose some valuable integration testing when dealing with exotic OSs like windows.

@marc-mabe
Copy link
Member Author

@Ocramius That's a good point! But we don't need to mock the fs everywhere in the tests - only when it's needed to do a good testing job. In the case of the timing testing issue it could be possible to mock the timing functions instead of the fs itself.

@marc-mabe
Copy link
Member Author

@Ocramius can this PR be merged even without a test?

@Ocramius
Copy link
Member

@marc-mabe testing this would be way too tricky without completely mocking out the FS. I think it's fine as-is.

@Ocramius Ocramius self-assigned this Nov 22, 2014
@Ocramius Ocramius added this to the 2.3.4 milestone Nov 22, 2014
@Ocramius Ocramius closed this in 2a68189 Nov 22, 2014
Ocramius added a commit that referenced this pull request Nov 22, 2014
…s-for-#6435' into develop

Close #6573
Close #6435
Forward port #6573
Forward port #6435
@Ocramius
Copy link
Member

Merged, thanks @marc-mabe!

master: 2a68189
develop: 11a6932

gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
boesing pushed a commit to laminas/laminas-cache-storage-adapter-apc that referenced this pull request May 24, 2020
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