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

Issue with list commands in 2.1 #158

Closed
djarnis73 opened this issue Aug 11, 2023 · 4 comments
Closed

Issue with list commands in 2.1 #158

djarnis73 opened this issue Aug 11, 2023 · 4 comments
Assignees
Labels
Milestone

Comments

@djarnis73
Copy link

djarnis73 commented Aug 11, 2023

Hi

After attempting to bump our claudb dependency from 2.0.1 to 2.1 one of our unit test started failing on what looks like a bug somewhere in lpush/rpush/lpop/rpop commands.

Simple reproducer (can be copied directly into ClauDBServerTest):

@Test
public void testPushPull() {
  execute(jedis -> {
    long push = jedis.rpush("key", "val1", "val2");
    String pop1 = jedis.rpop("key");
    String pop2 = jedis.rpop("key");
    assertThat(push, is(2l));
    assertThat(pop1, equalTo("val2"));
    assertThat(pop2, equalTo("val1"));
  });
}

It fails on the last assert where pop2 is null instead of expected val1.

I checked the changes and the lpush and rpush commands were both changed, so I would expect a bug was introduced here.

@tonivade tonivade self-assigned this Aug 11, 2023
@tonivade tonivade added the bug label Aug 11, 2023
@tonivade tonivade added this to the 2.1.1 milestone Aug 11, 2023
tonivade added a commit that referenced this issue Aug 11, 2023
tonivade added a commit that referenced this issue Aug 11, 2023
tonivade added a commit that referenced this issue Aug 11, 2023
tonivade added a commit that referenced this issue Aug 16, 2023
@tonivade tonivade linked a pull request Aug 17, 2023 that will close this issue
@tonivade
Copy link
Owner

tonivade commented Sep 4, 2023

hi @djarnis73 I released a snapshot for release 2.1.1, can you take a look if it works for you?

<dependency>
  <groupId>com.github.tonivade</groupId>
  <artifactId>claudb</artifactId>
  <version>2.1.1-SNAPSHOT</version>
</dependency>

@djarnis73
Copy link
Author

djarnis73 commented Sep 4, 2023

Hi @tonivade, I can confirm that our tests no longer fail with 2.1.1-SNAPSHOT. Thanks a lot for the fix.

On a side note, I would really love to see a dependency update of resp-server, to get rid of the netty-all transitive dependency.

@tonivade
Copy link
Owner

tonivade commented Sep 4, 2023

Ok, I will release version 2.1.1 ASAP

About resp-server without netty transitive dependencies. My idea is to create another release 2.2 with this change, and include the fix made in version 2.1.1.

@tonivade
Copy link
Owner

tonivade commented Sep 5, 2023

the new release 2.1.1 is available https://github.com/tonivade/claudb/releases/tag/2.1.1

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

Successfully merging a pull request may close this issue.

2 participants