test: add unit tests for secureMaxVersioned / secureMaxRemovalPolicy (fixes #44)#57
test: add unit tests for secureMaxVersioned / secureMaxRemovalPolicy (fixes #44)#57hanu-14 wants to merge 3 commits into
Conversation
alphacrack
left a comment
There was a problem hiding this comment.
Good table coverage of the ranking — the tighten-only direction is well exercised in both arguments. One gap to close before merge, since it's an explicit acceptance criterion in #44:
Missing: the unknown-rank fallback path. No case exercises a policy outside the DESTROY/SNAPSHOT/RETAIN ranking — e.g. RemovalPolicy.RETAIN_ON_UPDATE_OR_DELETE. Please add cases passing it as the tier value and as the incoming value, asserting the documented fallback behavior. That should take fields.ts branch coverage to 100%, which is the bar #44 sets.
Minor (non-blocking): should treat truthy/incoming coercion correctly repeats assertions from the two tests above it — fold it into them or drop it.
Also FYI: the diff currently includes #54/#56 content because the branches are stacked; that resolves itself as the stack merges bottom-up.
|
Status check: no new commits since the review, so this is still blocked on the one requested change — cases for the unknown-rank fallback path ( Heads-up on the environment: main moved to ESLint 10 + TypeScript 6 today (#59, #61). No action needed on your branch beyond the test cases — the lint rules were preserved 1:1 and your CI will simply run on the new toolchain after you push. |
|
Thanks for the heads-up on the review and the environment update. I’ll add test cases for the unknown-rank fallback path in |
Table-driven unit tests covering all combinations of tier x incoming values for the tighten-only merge helpers, including SNAPSHOT ranking.