-
Notifications
You must be signed in to change notification settings - Fork 366
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
ST-601 -- add randomized test for removal of keys from the state #1261
Conversation
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
assert val == int(state.get(key.encode(), isCommitted=False).decode()) | ||
state.remove(key.encode()) | ||
else: | ||
state.set(key.encode(), str(val).encode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we intentionally override existing key here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generate a new key while we generating data for test. This key differs from a key that is in state.
state/test/test_remove_from_trie.py
Outdated
|
||
@pytest.fixture() | ||
def rand(): | ||
return random.randint(0, 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need random here? Why don't we always deal with 1000 key-value pairs?
state.remove(key.encode()) | ||
else: | ||
state.set(key.encode(), str(val).encode()) | ||
for key, val in data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to check that removed values are not found in state
sequence, data = sequence | ||
for action, key, val in sequence: | ||
if action: | ||
assert val == int(state.get(key.encode(), isCommitted=False).decode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this test we never commit. It would be great to test both committed and uncommitted cases, that is (randomly) commit values, and make sure that remove still works.
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev nikita.khateev@dsr-corporation.com