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

add test for cache_version precision #34089

Merged
merged 1 commit into from
Oct 7, 2018

Conversation

lsylvester
Copy link
Contributor

Adds test to ensure that the cache_version supports subseconds on databases that support it. This should help preventing losing precision when optimising the cache keys in #33835.

Note that the additional touch in test_cache_key_format_is_precise_enough was added as the test was intermittently passing depending if the test was running after first second of test suite. And the skip for test_cache_key_format_is_not_too_precise was dropped because the test is still valid for when there is no sub-second support.

/cc @schneems

@rails-bot
Copy link

r? @kaspth

(@rails-bot has picked a reviewer for you, use r? to override)

@schneems
Copy link
Member

schneems commented Oct 5, 2018

Nice! Instead of ensuring that the value is false we should record the original value and set it back to that.

Also instead of relying on code execution timings we should put the calls in timecop blocks to guarantee timing.

@schneems
Copy link
Member

schneems commented Oct 6, 2018

I think this test is fixed on master. Can you rebase?

@schneems
Copy link
Member

schneems commented Oct 6, 2018

There's a few code climate issues

@lsylvester
Copy link
Contributor Author

@schneems I have fixed the code climate issues and squashed my commits. This branch should be good now.

@schneems schneems merged commit 01294c2 into rails:master Oct 7, 2018
@schneems
Copy link
Member

schneems commented Oct 7, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants