Skip to content

Make sure we use different cache directories every time we run the tests #205

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

Merged
merged 2 commits into from
Sep 4, 2017

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Sep 3, 2017

Q A
Bug fix? yes, kind of
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
Documentation
License MIT

What's in this PR?

When you run the tests you build some cache in your system's temp directory. If you say, bump some versions that cache may not be properly cleared. This fix make sure we always start with a clear cache every time we run the tests.

Copy link
Collaborator

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree for the sake of clean tests, but our code should be robust to not fail on outdated format or even corrupt files.

@@ -12,6 +12,11 @@ class AppKernel extends Kernel
use MicroKernelTrait;

/**
* @var string
*/
private static $cachePrefix;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: this is not just the prefix but the whole folder name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@fbourigault
Copy link
Contributor

StyleCI errors are fixed in #203.

@Nyholm
Copy link
Member Author

Nyholm commented Sep 4, 2017

The PR is updated and ready to be merged.

@Nyholm Nyholm merged commit 7206951 into master Sep 4, 2017
@Nyholm Nyholm deleted the patch-test-cache branch September 4, 2017 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants