Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ When working on a feature, read these paths first:
|-------|------------|
| Connection / options | `lib/valkey.rb` (`#initialize`), `test/lint/connection_options.rb` |
| New command | `lib/valkey/request_type.rb`, matching `lib/valkey/commands/*.rb`, `test/lint/*` |
| Testing | `docs/TESTING.md` |
| Pipelining / batch | `lib/valkey.rb` (`pipelined`, `send_batch_commands`), `lib/valkey/pipeline.rb` |
| OpenTelemetry | `lib/valkey/opentelemetry.rb`, `test/integration/valkey/opentelemetry_test.rb` |
| FFI / errors | `lib/valkey/bindings.rb`, `lib/valkey/errors.rb` |
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

### Changes

* Ruby: Strings: added support for `delifeq(key, value)`. Requires Valkey 9.0. ([#382](https://github.com/valkey-io/valkey-glide-ruby/pull/382))

* Ruby: Connection: added support for `Valkey::ReadFrom::AZ_AFFINITY_ALL_NODES`; String `client_az` values are trimmed, and all AZ-affinity strategies require a nonblank effective value. ([#349](https://github.com/valkey-io/valkey-glide-ruby/pull/349))

* Ruby: Hashes: added support for `hgetdel(key, *fields)`. Requires Valkey 9.1. ([#348](https://github.com/valkey-io/valkey-glide-ruby/pull/348))
Expand Down
61 changes: 61 additions & 0 deletions docs/TESTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Testing guidelines

This document is the authoritative testing reference for the Valkey GLIDE Ruby client. Follow these guidelines when developing new tests to keep them consistent, maintainable, and resilient to unrelated changes.

## Overall guidelines

- Avoid magic numbers. Use named variables, especially when a number has a unit.

## Test naming and organization

- Group tests by the feature they test.
- Within each group, order positive tests first, followed by negative tests and edge cases.
- Start with `test_<command_or_feature>`, which contains the basic positive-case assertions. Follow it with separate negative and edge-case tests.
- Use the format `test_<command_or_feature>_<condition_or_context>`.

Example:

```ruby
def test_get
# Assert the basic positive case.
end

def test_get_not_found
# Assert a simple negative case.
end

def test_get_nil
# Assert an edge case.
end

def test_delifeq
end

def test_delifeq_not_matching
end

def test_delifeq_binary_value
end
```

## Test coverage

- Unit and integration tests have different purposes and cover different parts of the client.
- Each assertion must verify behavior owned by the layer under test.

### Unit tests

- Unit tests cover Ruby-owned behavior, including request construction and data conversion before values reach Core.
- Each new component or API should have its own unit tests.
- Tests must assert only the component's properties and behavior.
- Tests should not cover Core or server behavior.
- Mocking and stubbing are allowed but should remain simple. If a test requires extensive mocking, consider skipping it and flagging the gap for human review.

### Integration tests

- Integration tests exercise the Ruby client from a user's point of view and verify public API contracts against a server.
- Negative tests must cover documented errors that are specific for that API; Generic errors like ACL and connections are not needed.
- Tests should assert return values and publicly observable state.
- They should not assert the Ruby client's private state. Unit tests own that behavior.
- Integration tests should use the Ruby client as an application would.
- Assertions must follow the public API documentation.
19 changes: 19 additions & 0 deletions lib/valkey/commands/string_commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,25 @@ def getdel(key)
send_command(RequestType::GET_DEL, [key])
end

# Delete a key only if its value matches the given value.
#
# @example
# valkey.set("key", "value")
# valkey.delifeq("key", "value")
# # => true
#
# @param [String] key
# @param [String] value the comparison value; the key is only deleted on an exact match
# @return [Boolean] true when the key was deleted; false when the key does not
# exist or its value did not match
#
# @see https://valkey.io/commands/delifeq/
#
# Since Valkey 9.0.
def delifeq(key, value)
Comment thread
currantw marked this conversation as resolved.
Resolved
send_command(RequestType::DEL_IF_EQ, [key, value.to_s])
end

# Get the value of key and optionally set its expiration.
#
# @param [String] key
Expand Down
3 changes: 2 additions & 1 deletion lib/valkey/commands/transaction_commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ def discard
BOOLEAN_REQUEST_TYPES = [
RequestType::EXPIRE, RequestType::EXPIRE_AT, RequestType::PEXPIRE, RequestType::PEXPIRE_AT,
RequestType::PERSIST, RequestType::SISMEMBER, RequestType::S_MOVE, RequestType::PFADD,
RequestType::RENAME_NX, RequestType::MOVE, RequestType::COPY, RequestType::MSET_NX
RequestType::RENAME_NX, RequestType::MOVE, RequestType::COPY, RequestType::MSET_NX,
RequestType::DEL_IF_EQ
Comment thread
Aryex marked this conversation as resolved.
Resolved
].freeze

private
Expand Down
1 change: 1 addition & 0 deletions lib/valkey/request_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ module RequestType
SET_RANGE = 1520
STRLEN = 1521
SUBSTR = 1522
DEL_IF_EQ = 1523

# Transaction commands
DISCARD = 1601
Expand Down
47 changes: 47 additions & 0 deletions test/lint/string_commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,45 @@ def test_getdel
end
end

def test_delifeq
target_version("9.0")

assert_delifeq(key: "key", value: "value")
assert_delifeq(key: "non-string", value: %w[a b])
assert_delifeq(key: "binary", value: "\x00\xFF".b)
end

def test_delifeq_not_matching
target_version("9.0")

assert_equal false, r.delifeq("foo", "value")

r.set("foo", "value")
assert_equal false, r.delifeq("foo", "other")
assert_equal "value", r.get("foo")
end

def test_delifeq_raises
target_version("9.0")

r.sadd("foo", "member")

assert_raises(Valkey::CommandError) { r.delifeq("foo", "value") }
end

def test_delifeq_in_pipeline
Comment thread
currantw marked this conversation as resolved.
Resolved
target_version("9.0")

r.set("foo", "value")

result = r.pipelined do |pipeline|
pipeline.delifeq("foo", "value")
pipeline.delifeq("foo", "other")
end

assert_equal [true, false], result
end

def test_getset
r.set("foo", "bar")

Expand Down Expand Up @@ -424,5 +463,13 @@ def test_lcs
]
], "len" => 6 }, r.lcs('{1}key1', '{1}key2', idx: true, min_match_len: 4, with_match_len: true))
end

private

def assert_delifeq(key:, value:)
r.set(key, value)
assert_equal true, r.delifeq(key, value)
assert_nil r.get(key)
end
end
end
36 changes: 36 additions & 0 deletions test/lint/transaction_commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,42 @@ def test_multi_with_setnx
assert_equal [false, true], result
end

def test_multi_exec_boolean_coercion
skip("MULTI/EXEC not supported in cluster mode") if cluster_mode?
target_version("9.0")

r.set("expire", "value")
r.set("expireat", "value")
r.set("pexpire", "value")
r.set("pexpireat", "value")
r.set("persist", "value", ex: 60)
r.sadd("sismember", "member")
r.sadd("smove-source", "member")
r.set("renamenx-source", "value")
r.set("move", "value")
r.set("copy-source", "value")
r.set("delifeq", "value")

r.multi
r.expire("expire", 60)
r.expireat("expireat", Time.now.to_i + 60)
r.pexpire("pexpire", 60_000)
r.pexpireat("pexpireat", (Time.now.to_f * 1000).to_i + 60_000)
r.persist("persist")
r.sismember("sismember", "member")
r.smove("smove-source", "smove-destination", "member")
r.pfadd("pfadd", "member")
r.renamenx("renamenx-source", "renamenx-destination")
r.move("move", 14)
r.copy("copy-source", "copy-destination")
r.msetnx("msetnx", "value")
r.delifeq("delifeq", "value")

result = r.exec
assert_equal Valkey::Commands::TransactionCommands::BOOLEAN_REQUEST_TYPES.size, result.size
assert result.all? { |value| [true, false].include?(value) }, "Expected only booleans, got #{result.inspect}"
end

def test_multi_with_block_returns_futures_that_resolve_after_the_block
skip("MULTI/EXEC not supported in cluster mode") if cluster_mode?

Expand Down
5 changes: 3 additions & 2 deletions test/support/helper/generic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ def assert_numsub(expected, actual)
def target_version(target)
if version < target
skip("Requires Valkey > #{target}") if respond_to?(:skip)
else
yield
return
end

yield if block_given?
end

def keys(pattern = "*")
Expand Down
63 changes: 63 additions & 0 deletions test/unit/commands/string_commands_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

require "minitest/autorun"
require "valkey/errors"
require "valkey/request_type"
require "valkey/commands/string_commands"

# Unit tests for string command request construction.
class TestStringCommandsUnit < Minitest::Test
Comment thread
currantw marked this conversation as resolved.
Resolved
# Minimal command receiver that records dispatched requests.
class FakeClient
include Valkey::Commands::StringCommands

attr_reader :captured_command, :captured_arguments

def initialize(reply: nil, error: nil)
@reply = reply
@error = error
end

def send_command(request_type, command_arguments = [])
@captured_command = request_type
@captured_arguments = command_arguments
raise @error if @error

@reply
end
end

def test_delifeq
assert_delifeq(key: "key", value: "value", expected_sent_value: "value")
end

def test_delifeq_non_string_value
# non-string values are stringified
assert_delifeq(key: "key", value: %w[a b], expected_sent_value: "[\"a\", \"b\"]")
end

def test_delifeq_stringifies_nil_value
Comment thread
currantw marked this conversation as resolved.
Resolved
assert_delifeq(key: "key", value: nil, expected_sent_value: "")
end

def test_delifeq_propagates_client_error
Comment thread
currantw marked this conversation as resolved.
Resolved
error = Valkey::CommandError.new("error")
client = FakeClient.new(error: error)

raised_error = assert_raises(Valkey::CommandError) { client.delifeq("key", "value") }

assert_same error, raised_error
end

private

def assert_delifeq(key:, value:, expected_sent_value:)
client = FakeClient.new(reply: 0)

assert_equal 0, client.delifeq(key, value)
assert_equal Valkey::RequestType::DEL_IF_EQ, client.captured_command
assert_equal [key, expected_sent_value], client.captured_arguments

client
end
end
Loading