Skip to content

Add add_test_hook() to run a hook around each function test - #262

Open
fcostaoliveira wants to merge 4 commits into
RedisLabsModules:masterfrom
fcostaoliveira:feat/test-hooks
Open

fcostaoliveira wants to merge 4 commits into
RedisLabsModules:masterfrom
fcostaoliveira:feat/test-hooks

Conversation

@fcostaoliveira

@fcostaoliveira fcostaoliveira commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Adds RLTest.add_test_hook(hook). hook(env, test) returns a context manager that RLTest enters before, and exits after, every function test that takes an env. test is the test function, so a hook can read markers set on it, such as an opt-out. Failed assertions and exceptions in the hook fail that test, as the test's own would. Hooks run in the order added.

RLTest re-imports a test file before each of its tests. So a top-level function added again with the same module and name replaces the earlier copy, and runs once per test. Any other callable, such as a lambda, closure or bound method, is the same hook only if it is the same object.

This lets a suite check every test in one place without patching RLTest's loader. memtier_benchmark uses it to fail any test that leaves an -NOAUTH reply on a password-protected server (redis/memtier_benchmark#863).

  • RLTest/test_hooks.py: the registry and run_with_hooks(), called from _runTest.
  • tests/unit/test_test_hooks.py: order, dedup across a re-import (the newer copy wins), distinct same-named hooks all running, exit on a failing test, a failing hook, and the test function being passed.
  • tests/flow/test_hooks_flow.py: through the runner, a hook added at a test file's top level runs exactly once per test.
  • README: a short example.

🤖 Generated with Claude Code

fcostaoliveira and others added 2 commits October 25, 2026 07:32
A hook is a callable that takes the env and returns a context manager.
It is entered before each function test that takes an env and exited
after it. Failed assertions and exceptions in the hook fail the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A hook can then honour markers set on the test, such as an opt-out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fcostaoliveira added a commit to redis/memtier_benchmark that referenced this pull request Oct 25, 2026
Instead of wrapping RLTest.loader.TestFunction.initialize, register the
check as a test hook. add_test_hook() is new in the RLTest fork commit
pinned here (RedisLabsModules/RLTest#262, on top of #253). The hook gets
the test function, so @allow_noauth still opts a test out.
test_guard_is_installed_in_rltest now runs a test through RLTest's hook
runner and expects the guard to fail it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 3f3e542. Configure here.

Comment thread RLTest/test_hooks.py Resolved
@codecov-commenter

codecov-commenter commented Oct 25, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 51.64%. Comparing base (02cfbb9) to head (d9eda2a).
⚠️ Report is 37 commits behind head on master.

Files with missing lines Patch % Lines
RLTest/__main__.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #262       +/-   ##
===========================================
+ Coverage   32.46%   51.64%   +19.18%     
===========================================
  Files          17       19        +2     
  Lines        2597     2864      +267     
===========================================
+ Hits          843     1479      +636     
+ Misses       1754     1385      -369     
Flag Coverage Δ
unittests 51.64% <95.65%> (+19.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

RLTest re-imports a test file before each of its tests, so a hook added
there was a new function each time and ran once more per test. Treat a
hook with the same module and name as the same hook. A flow test checks,
through the runner, that such a hook runs once per test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fcostaoliveira added a commit to redis/memtier_benchmark that referenced this pull request Oct 25, 2026
73c10b7 keeps one copy of a test hook that a test file adds again each
time RLTest re-imports it. memtier registers its hook from include.py,
which is imported once, so it was not affected; this keeps the pin on
the reviewed version of RedisLabsModules/RLTest#262.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lambdas, closures from one factory and bound methods share names while
being different hooks, so keying them by name dropped all but the last.
Match by module and name only a top-level function, which is what a
re-imported test file adds again, and anything else by identity. The
unit tests now import a test file twice, check distinct same-named hooks
all run, and check a hook exits with the test's exception.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fcostaoliveira added a commit to redis/memtier_benchmark that referenced this pull request Oct 25, 2026
…863)

With REDIS_PASSWORD set, a test that started memtier or a redis-py client
without credentials got -NOAUTH replies, and nothing failed it. #842
found tests that silently ran unauthenticated.

Check every server of the env, masters and replicas, for new
errorstat_NOAUTH around each function test. The check runs as an RLTest
test hook (add_test_hook, RedisLabsModules/RLTest#262). The RLTest pin
moves to a fork commit with that hook on top of #253's cluster-aware
replicas. A test that is unauthenticated on purpose opts out with
@allow_noauth(reason). The guard fails the test rather than pass when
it can read no server before it, and an unexpected error while reading
a server (an RLTest API change, say) raises instead of leaving the
server unchecked. Replicas get the env password too.

shutdown_nosave() reports only a real stop. Its clients, and the
guard's, go through bounded_client(), which turns off redis-py retries,
including on connections the client already has open, so a refused or
successful SHUTDOWN returns at once instead of after seconds of retry
backoff, and is never resent. The 3-shard replica failover file with a
password takes about 70 s instead of 132 s. Raise the redis-py floor to
6.0, which the harness tests need.

test_harness_auth.py covers the auth-argument forwarding, the guard's
pass, fail, opt-out and unreadable-server paths, its installation in
RLTest, bounded_client and shutdown_nosave.

Fixes #842

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants