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

Allow injection of clock into ExpiringMemoizationSupplier to allow for testing #2727

Closed
mokah opened this issue Jan 27, 2017 · 1 comment
Closed
Assignees
Labels
P4 package=base status=will-not-fix type=other Miscellaneous activities not covered by other type= labels

Comments

@mokah
Copy link

mokah commented Jan 27, 2017

To allow testing of ExpiringMemoizationSupplier, a second constructor with @VisibleForTesting should be added that allows the injection of a clock.

Right now it is being tested by calling Thread.sleep(x) but an injection of a clock would be a better way to test, I believe. Thoughts?

@cgdecker
Copy link
Member

It could be a slight improvement, perhaps, but I'm not sure it's worth it in this case.

The main reason I say that is because ExpiringMemoizingSupplier is Serializable, and changing it to have a Ticker field to allow for this testing would also require making our system time Ticker serializable... I don't know that that's something we'd want to do, because it doesn't have any state or data to actually serialize and instead just relies on the system clock.

@cgdecker cgdecker added status=will-not-fix type=other Miscellaneous activities not covered by other type= labels P4 labels Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 package=base status=will-not-fix type=other Miscellaneous activities not covered by other type= labels
Projects
None yet
Development

No branches or pull requests

6 participants