Conversation
kevinlind
left a comment
There was a problem hiding this comment.
Looks good. One minor comment, for the positive test cases where the state store does not expire, you could consider increasing the timeout to avoid false test failures on slow CI systems.
| value: "or2", | ||
| maxAge: 1800 | ||
| }, | ||
| "expiryTs": _adb_timestampInMillis() + 1000 |
There was a problem hiding this comment.
nit: Consider making the timeout larger than 1 second in case of slow CI systems to avoid false negative. Maybe set it to 3 or 5 seconds?
There was a problem hiding this comment.
These tests don't run on CI as there is no roku device support.
code/main/edge/stateStoreManager.brs
Outdated
| maxAgeSeconds = payload.maxAge | ||
| if not _adb_isPositiveWholeNumber(maxAgeSeconds) | ||
| _adb_logDebug("_adb_StateStore::_setExpiryTime() - Invalid payload.maxAge value:(" + FormatJson(maxAgeSeconds) + "), using 0 as default value.") | ||
| maxAgeSeconds = 0 |
There was a problem hiding this comment.
Is there a default max age? You can remove the entry instead of setting its age to 0, as it will simply expire during the next call.
There was a problem hiding this comment.
@praveek Updated the logic to explicitly delete the entry.
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: