Skip to content
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

Http Integration Tests: Create tests to support HTTP(5) #738

Closed
pratikpandey21 opened this issue Sep 26, 2024 · 8 comments · Fixed by #764
Closed

Http Integration Tests: Create tests to support HTTP(5) #738

pratikpandey21 opened this issue Sep 26, 2024 · 8 comments · Fixed by #764
Assignees

Comments

@pratikpandey21
Copy link
Contributor

This issue involves creating the below test cases for HTTP as well:

Source: integration_tests/commands/async/

  • flushdb_test.go
  • get_test.go
  • getdel_test.go
  • getex_test.go
  • getset_test.go

Steps:

  1. Create the corresponding tests for these commands in integration_tests/commands/http.
  2. Use set_test.go as a reference, and how to alter the input and output to match JSON structures from RESP.
  3. Run the tests and fix issues with command parsing in HTTP.

If you need to do anything custom to support HTTP, let @pratikpandey21 @lucifercr07 know.

@karandixit10
Copy link
Contributor

@pratikpandey21 can I take this up?

@pratikpandey21
Copy link
Contributor Author

I don't have the permissions to assign you, but please go ahead and start @karandixit10

@karandixit10
Copy link
Contributor

@pratikpandey21 for FLUSHDB command will the body be empty:

{Command: "FLUSHDB", Body: map[string]interface{}{}},

@pratikpandey21
Copy link
Contributor Author

Yes, for commands that don't need arguments, it will be empty.

@karandixit10
Copy link
Contributor

karandixit10 commented Sep 27, 2024

@pratikpandey21
Is SADD command inconsistent it's returning 1 first and that to a float64.
image

And it's passing test for this when expecting float64(2) as response.
image

Not sure why this is happening?

@pratikpandey21
Copy link
Contributor Author

Isn't that how redis will behave?

First SADD: If the value is not yet in the set, Redis will add the value to the set and return 1, indicating that one element was successfully added.

  1. Second SADD with the same value: Redis sets are collections of unique values, so the second SADD with the same value will not add a duplicate to the set. Instead, Redis will return 0, indicating that no new element was added since the value was already present.

If you have confusion regarding behavior of any command, please validate behavior in redis.

Float is because of JSON parsing, you could reference existing tests for the same

@karandixit10
Copy link
Contributor

Isn't that how redis will behave?

First SADD: If the value is not yet in the set, Redis will add the value to the set and return 1, indicating that one element was successfully added.

  1. Second SADD with the same value: Redis sets are collections of unique values, so the second SADD with the same value will not add a duplicate to the set. Instead, Redis will return 0, indicating that no new element was added since the value was already present.

If you have confusion regarding behavior of any command, please validate behavior in redis.

Float is because of JSON parsing, you could reference existing tests for the same

Thanks for clarifying this, I'll refer to the redis behavior also.

@pratikpandey21
Copy link
Contributor Author

And why adding member and member 1, returns 2 is because the HTTP parser doesn't know how to handle "member".

Please check how the parsing is working for HTTP requests in rediscmdadapter file. You will need to add member as a priority key and that should make it work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants