DOC-7011 Stub SkipIfRedisFactAttribute so nredisstack cmds_generic compiles - #3895
Merged
Merged
Conversation
…mpiles The portable C# harness's stubs.cs stands in for NRedisStack's own test fixtures so a doc example can run under plain xunit. It covered AbstractNRedisStackTest and [SkippableFact] but not [SkipIfRedisFact(...)], which cmds_generic's example uses to skip on old servers — so nredisstack failed to compile (CS0246) on that one set. Added a minimal Comparison enum and a no-op SkipIfRedisFactAttribute, matching the existing SkippableFact stub's philosophy: the harness always runs against a recent Redis, so there's no need to reproduce NRedisStack's real version-detection logic. Upstream's actual attribute (checked against a local NRedisStack clone) has moved twice: an older SkipIfRedisAttribute, then today's much larger SkipIfRedisFactAttribute built on custom xunit v3 discoverers. Neither is worth mirroring here — the stub only has to satisfy the one constructor overload docs examples actually call. cmds_hash, cmds_stream, geoindex, search_quickstart and time_series_tutorial still FAIL for nredisstack under this harness, but for unrelated reasons confirmed by their error messages: the ambient local Redis (7.2.7, no modules) doesn't have HEXPIRE, FT.CREATE, or TS.CREATE. Not a regression from this change and not in scope for this ticket. Learned: the doc example's [SkipIfRedisFact] attribute name doesn't match any class in an older local NRedisStack clone (SkipIfRedisAttribute) — upstream has renamed/rebuilt this fixture at least twice; don't assume a name in a doc example still matches the current upstream source Constraint: keep stubs.cs minimal — it only needs to satisfy the constructor overloads doc examples actually call ([SkipIfRedisFact(Comparison.LessThan, "7.0.0")] today), not reproduce NRedisStack's real skip/version logic Gaps: cmds_hash/cmds_stream/geoindex/search_quickstart/time_series_tutorial still fail nredisstack under --portable — the ambient Redis lacks HEXPIRE and the search/timeseries modules, unrelated to this fix Ticket: DOC-7011
Contributor
Contributor
🧠 Redis MemoryFound 5 related items from repository history (5 new this commit):
Memory updated at 4b3a228 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DOC-7011
What
The portable C# test harness (
build/example-test-harness/dotnet/stubs.cs) stands in for NRedisStack's own test fixtures so a doc example can compile and run under plain xunit. It stubbedAbstractNRedisStackTestand[SkippableFact], but not[SkipIfRedisFact(...)]— whichcmds_generic's NRedisStack example uses to skip on old servers. That left nredisstack failingcmds_genericwithCS0246: SkipIfRedisFactAttribute could not be found.Added a minimal
Comparisonenum and a no-opSkipIfRedisFactAttribute, following the same philosophy as the existingSkippableFactstub: the harness always runs against a recent Redis, so there's no need to reproduce NRedisStack's real version-detection logic — just satisfy the one constructor overload the doc example actually calls.A wrinkle worth flagging
Checked a local NRedisStack clone for the "real" implementation to model the stub on, and found the attribute has moved twice upstream: an older
SkipIfRedisAttribute, then today'smasterhas a much largerSkipIfRedisFactAttributebuilt on custom xunit v3 discoverers. Neither is worth mirroring — the stub stays deliberately minimal.Verification
build/example-test-harness/run.sh cmds_generic nredisstack→ PASS (was a compile failure before).cmds_string nredisstack(also uses the shared stub file, not this attribute) → PASS, confirming the stub file itself is still intact.cmds_hash,cmds_stream,geoindex,search_quickstart,time_series_tutorial): all still FAIL, but for unrelated reasons visible in their error messages — the ambient local Redis (7.2.7, no modules) doesn't haveHEXPIRE,FT.CREATE, orTS.CREATE. Not a regression from this change, and out of scope for this ticket.🤖 Generated with Claude Code
Note
Low Risk
Changes only affect example-test-harness stubs for documentation builds; no runtime product or security paths.
Overview
The portable C# doc example harness in
stubs.csnow defines a minimalComparisonenum andSkipIfRedisFactAttribute(constructor overload matching the docs), both inheriting from plain xunitFactbehavior like the existingSkippableFactstub.This fixes
cmds_genericNRedisStack examples that annotate tests with[SkipIfRedisFact(Comparison.LessThan, "7.0.0")], which previously failed to compile withCS0246. Version-based skipping is intentionally not implemented because the harness assumes a recent Redis.Reviewed by Cursor Bugbot for commit 4b3a228. Bugbot is set up for automated code reviews on this repo. Configure here.