Migrate Commands: ('SETBIT', 'GETBIT', 'BITCOUNT', 'BITPOS', 'BITFIELD', 'BITFIELD_RO')

[vishnuchandrashekar]

Migrates the eval functions for SETBIT, GETBIT, BITPOS, BITFIELD and BITFIELD_RO to the new eval function type independent of protocol.
Closes #1017

Checklist:

• Migrated the evalXXX function with the latest definition
• Update or add unit tests for the new implementation.
• All unit tests pass successfully.
• Ensure all integration tests pass successfully.
• Ensure integration tests are added for the migrated command on multi-threaded resp server.
• Move Integration tests for the respective commands under the RESP integration tests directory from Async directory
• Please validate that the documentation for the respective commands is up to date. If not then consider adding them.

Notes:
Since the BITFIELD and BITFIELD_RO commands depend on the same function (bitfieldEvalGeneric) I have gone ahead and migrated both the commands.

[AshwinKul28]

Huge changes. Thanks a lot for this @vishnuchandrashekar . Left few minor comments please check and also check CI runs.

[AshwinKul28]

HI @vishnuchandrashekar thanks a lot for the changes. Looks good. Please rebase your branch with master to resolve conflicts.

[soumya-codes]

@vishnuchandrashekar lets ensure that the corresponding integration tests are migrated as well. cc @AshwinKul28

I have added the checklist to the PR description. Please fill the same.
Please let me or Ashwin know if you have any doubts/concern.

[AshwinKul28]

HI @vishnuchandrashekar hope you are doing well. Do you still have any blockers on this PR? please let me or others know on the Discord.

[vishnuchandrashekar]

Hey @AshwinKul28, I've moved all the tests related to BIT to the file integration_tests/commands/resp/bit_test.go. Is this fine or do I need to do it the same way as it was done in async?

[apoorvyadav1111]

Hi @vishnuchandrashekar , you can rename the file. Additionally, we do not required the async test cases anymore, if you have migrated all the tests to resp, please remove the async tests. Additionally, please add tests in websocket and http folder.

Thanks

[vishnuchandrashekar]

Hey @AshwinKul28 / @apoorvyadav1111, some of the websocket and http tests seems to be failing. I am unsure as to why. Any suggestions?

[apoorvyadav1111]

Hi @vishnuchandrashekar, I checked the lint run but there I can only see unit tests failing, Could you please share logs of failing websocket and HTTP tests on discord and tag me.
Thanks

[vishnuchandrashekar]

Hi @apoorvyadav1111, apologies I meant the tests that I added. Mainly the http BITPOS tests, could you have a look at those? I'll share the logs on discord soon. Thanks.
EDIT: The same tests pass for resp and websocket. Maybe I'm doing something wrong in the http tests?

[apoorvyadav1111]

Yes, please share your logs on discord. Thanks

[JyotinderSingh]

Hi @vishnuchandrashekar are you still blocked on this issue?

[vishnuchandrashekar]

Hi @vishnuchandrashekar are you still blocked on this issue?

Hi @JyotinderSingh, yes I'm still facing issues. I've sent a message on discord. Currently working on updating/adding the documentation for the commands.

[AshwinKul28]

HI @vishnuchandrashekar Changes look great. Please have a look at this reference doc for the documentation changes. I see a few tests are failing, consider fixing them as well. Again, thanks a lot for the commendable efforts

[AshwinKul28]

HI @vishnuchandrashekar hope you are doing great. Can you please have a look at the documentation part as mentioned above? Also, is it possible to close this as early as possible? since we need to make DiceDB production ready soon for the upcoming major version release.

Also more delay causes more conflicts 😭 to resolve as well. Let me know if you need any sort of help. Thanks again!

[vishnuchandrashekar]

Hey @AshwinKul28, I'll make sure to push the changes by the end of the day. Thanks for your patience.

[vishnuchandrashekar]

Hey @AshwinKul28, some of the integration tests that I've added are failing namely:

HTTP:

1. TestBitOpsString/Bitop_not_of_a_key_containing_a_string
2. TestBitOpsString/Bitop_not_of_a_key_containing_an_integer
3. TestBitOpsString/BITOP_OR_of_keys_containing_strings_and_a_bytearray_and_get_the_destkey

RESP:

1. TestBitOpsString/BITOP_XOR_of_keys_containing_strings_and_get_the_destkey
2. TestBitOpsString/BITOP_XOR_of_keys_containing_strings_and_a_bytearray_and_get_the_destkey

Websocket:

1. TestBitOpsString/Bitop_not_of_a_key_containing_a_string
2. TestBitOpsString/Bitop_not_of_a_key_containing_an_integer
3. TestBitOpsString/BITOP_OR_of_keys_containing_strings_and_a_bytearray_and_get_the_destkey

Also I am unsure as to why the build tests are failing, the tests pass on my local.
I am currently looking into this, would like your suggestions.

[vishnuchandrashekar]

Hey @AshwinKul28, I've fixed some of the failing test cases. I am still unsure why the RESP tests fail, would like your inputs.
@lucifercr07 - The http and websocket had some marshalling issues while returning the data, I've come up with a quick work around. Let me know what you think. I'm sure there is a better way to fix this.

[apoorvyadav1111]

Hi @vishnuchandrashekar , #1245 is closed. Lets move ahead.

[apoorvyadav1111]

.....faaantastic work! Thanks for such a great effort.