-
Notifications
You must be signed in to change notification settings - Fork 65
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
added hexists command in node. #489
Conversation
node/src/BaseClient.ts
Outdated
* | ||
* @param key - The key of the hash. | ||
* @param field - The field to check in the hash stored at key. | ||
* @returns 1 if the hash contains field. 0 if the hash does not contain field, or key does not exist. |
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.
key does not exist
is... what? an exception? a string?
if it's an exception, it isn't returned, it's thrown. do we have such an exception?
ee8ba21
to
cc3c575
Compare
cc3c575
to
4b99d20
Compare
node/src/BaseClient.ts
Outdated
* | ||
* @param key - The key of the hash. | ||
* @param field - The field to check in the hash stored at key. | ||
* @returns 1 if the hash contains field. If the hash does not contain the field, or if the key does not exist, it returns 0. |
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.
* @returns 1 if the hash contains `field`. If the hash does not contain `field`, or if `key` does not exist, returns 0.
Make sure to use `` on variable names
node/src/Transaction.ts
Outdated
* @param key - The key of the hash. | ||
* @param field - The field to check in the hash stored at key. | ||
* | ||
* Command Response - 1 if the hash contains field. If the hash does not contain the field, or if the key does not exist, |
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.
same fix here
node/tests/SharedTests.ts
Outdated
const nonExistingKey = uuidv4(); | ||
const field1 = uuidv4(); | ||
const field2 = uuidv4(); | ||
const nonExistingField = uuidv4(); |
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.
You don't need to generate random uuids for values / non existing fields if they don't change the database/not being tested, you can use simple strings instead, e.g.:
const key = uuidv4();
const field1 = uuidv4();
const field2 = uuidv4();
const fieldValueMap = {
[field1]: "value1",
[field2]: "value2",
};
expect(await client.hset(key, fieldValueMap)).toEqual(2);
expect(await client.hexists(key, field1)).toEqual(1);
expect(await client.hexists(key, "non-existing-filed")).toEqual(0);
expect(await client.hexists("non-existing-key", field2)).toEqual(0);
We use random strings for keys to make sure that if we run multiple tests at the same time, we don't accidentally use the same keys in different tests to prevent one test from messing with another. So if there's no risk of collision you can use simple strings
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.
FSM
b882338
to
a4ca89f
Compare
a4ca89f
to
afbd632
Compare
added hexists command in node.