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

Add failing test for nonexistent Redis commands #161

Merged
merged 1 commit into from
Jun 3, 2019
Merged

Add failing test for nonexistent Redis commands #161

merged 1 commit into from
Jun 3, 2019

Conversation

jbmeerkat
Copy link
Contributor

There are no [] and []= commands in Redis, but redis-namespace responds to those methods. If you try to call one of them — Redis will fail with "Redis::CommandError: ERR unknown command '[]'" or "Redis::CommandError: ERR unknown command '[]'" depending of the command.

Copy link
Contributor

@jeremy jeremy left a comment

Choose a reason for hiding this comment

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

When were these methods removed from redis-rb? Does redis-namespace support versions of redis-rb that still implement these methods?

@jbmeerkat
Copy link
Contributor Author

jbmeerkat commented May 30, 2019

@jeremy for now, redis-namespace supports version 3.0.4 and greater. I've checked versions v4.0.0, v3.0.0, v2.0.0, v1.0.0 and v0.2.0 of redis-rb and none of them responds to :[].

Code was added in 1e8a532 and I see no reasons why [] and []= are there. Fork, which @defunkt mentioned, doesn't exist anymore, so the only person who knows anything about it is Chris.

@jeremy
Copy link
Contributor

jeremy commented May 31, 2019

Sounds good, @jbmeerkat. Thanks for verifying! Let's go ahead and remove the failing tests, then 😊

redis-namespace delegates `[]` and `[]=` to redis-rb client, but it
doesn't respond to these methods. All the previous versions down to 0.2.0 doens't respond too.

Removed `[]` and `[]=` delegation.
@jbmeerkat
Copy link
Contributor Author

@jeremy removed test and squashed in one commit. Should I also update changelog or something?

@jeremy jeremy merged commit 9f5ba34 into resque:master Jun 3, 2019
@jeremy
Copy link
Contributor

jeremy commented Jun 3, 2019

Thanks @jbmeerkat! We'll manage that as part of the next release.

@jbmeerkat jbmeerkat deleted the invalid-redis-commands branch June 5, 2019 09:09
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.

None yet

2 participants