Skip to content

Fix new subscribe mode test in reply-schemas-validator#11939

Merged
oranagra merged 3 commits intoredis:unstablefrom
enjoy-binbin:fix_new_subscribe_cli_test
Mar 20, 2023
Merged

Fix new subscribe mode test in reply-schemas-validator#11939
oranagra merged 3 commits intoredis:unstablefrom
enjoy-binbin:fix_new_subscribe_cli_test

Conversation

@enjoy-binbin
Copy link
Contributor

@enjoy-binbin enjoy-binbin commented Mar 20, 2023

The reason is in reply-schemas-validator, the resp of the
client we create will be client_default_resp (currently 3):

client *createClient(connection *conn) {
    client *c = zmalloc(sizeof(client));
 #ifdef LOG_REQ_RES
    reqresReset(c, 0);
    c->resp = server.client_default_resp;
 #else
    c->resp = 2;
 #endif
}

But current_resp3 in redis-cli will be inconsistent with it,
the test adds a simple hello 3 to avoid this failure, test
was added in #11873.

Added help descriptions for dont-pre-clean option, it was
added in #10273

The reason is in reply-schemas-validator, the resp of the
client we create will be client_default_resp (currently 3):
```
client *createClient(connection *conn) {
    client *c = zmalloc(sizeof(client));
 #ifdef LOG_REQ_RES
    reqresReset(c, 0);
    c->resp = server.client_default_resp;
 #else
    c->resp = 2;
 #endif
}
```

But current_resp3 in redis-cli will be inconsistent with it,
the test adds a simple hello 3 to avoid this failure, test
was added in redis#11873.

Added help descriptions for log-req-res, force-resp3 and
dont-pre-clean options, options was added in redis#10273
@enjoy-binbin
Copy link
Contributor Author

enjoy-binbin commented Mar 20, 2023

https://github.com/redis/redis/actions/runs/4463382058/jobs/7838545346

*** [err]: Interactive CLI: Subscribed mode in tests/integration/redis-cli.tcl
Expected '1) "subscribe"
2) "ch1"
3) (integer) 1
1) "subscribe"
2) "ch2"
3) (integer) 2
1) "subscribe"
2) "ch3"
3) (integer) 3
Reading messages... (press Ctrl-C to quit or any key to type command)
' to be equal to '1) "subscribe"
2) "ch1"
3) (integer) 1
1) "subscribe"
2) "ch2"
3) (integer) 2
1) "subscribe"
2) "ch3"
3) (integer) 3' (context: type eval line 9 cmd {assert_equal $sub1$sub2$sub3$reading  [run_command $fd "subscribe ch1 ch2 ch3"]} proc ::test)

I chose the easiest way to fix it. did not modify the assignment of current_resp3 in redis-cli (@zuiderkwast FYI. or maybe you have a better way... ), because this adds a bit of code complexity, and LOG_REQ_RES should only be used for our tests for now

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case LGTM

Thanks for fixing!

@oranagra oranagra merged commit c912414 into redis:unstable Mar 20, 2023
@enjoy-binbin enjoy-binbin deleted the fix_new_subscribe_cli_test branch March 20, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants