You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
For testing purposes it is neat to overwrite the clock of UTCInstant. Currently, the used clock is static, which brings its various problems with it. Also the function setClock overwrites the clock for all future instances, which might lead to unexpected behaviour in case of forgotten resetClock. The latter can be prevented by using the proper Disposable feature of the UTCInstant class.
However, we should overthink the architecture and discuss the issue here, rather than in #215 since it does not pose an immediate risk and should no prevent he PR from merging (it only appears in unit-tests and integration-tests).
The text was updated successfully, but these errors were encountered:
Another code-smell is the fact that static variables are kind of a hidden global variable. And those are never good, because they make it harder to modularize and test things. If for example you want to test clock-skews of different clients, you have to take care that you don't overwrite the clocks from the different clients and the backend.
In addition to that, UTCInstant has a double-function:
getting the actual time and date
converting different time/duration representations from/to each other
The minimum change IMO is to:
change private static Clock to protected static Clock
move implements Closeable, setClock, resetClock and close from UTCInstant to UTCITest
make UTCITest only available in the tests
This would make sure that nobody calls setClock in the main code.
For testing purposes it is neat to overwrite the clock of UTCInstant. Currently, the used clock is static, which brings its various problems with it. Also the function
setClock
overwrites the clock for all future instances, which might lead to unexpected behaviour in case of forgottenresetClock
. The latter can be prevented by using the properDisposable
feature of theUTCInstant
class.However, we should overthink the architecture and discuss the issue here, rather than in #215 since it does not pose an immediate risk and should no prevent he PR from merging (it only appears in unit-tests and integration-tests).
The text was updated successfully, but these errors were encountered: