Skip to content

Conversation

@UdeshyaDhungana
Copy link
Contributor

@UdeshyaDhungana UdeshyaDhungana commented Nov 24, 2025

  • Previously:
    • Tester panicked if the resp value is an empty array
  • Now:
    • Tester panics if the assertion specification is an empty array (intended)

Source: https://forum.codecrafters.io/t/redis-auth-test-run-crashes-gx8/15564


Note

ArrayElementsAssertion now panics only when the specification list is empty, and the field is pluralized with all call sites updated.

  • resp_assertions:
    • Change panic condition in ArrayElementsAssertion.Run to trigger when ArrayElementAssertionSpecifications is empty (was panicking on empty array value).
    • Rename field to ArrayElementAssertionSpecifications and update all internal references (MaxFunc, SortFunc, iteration).
  • test_cases:
    • Update AclGetuserTestCase to use ArrayElementAssertionSpecifications and adjust all appends accordingly.

Written by Cursor Bugbot for commit d97fbd9. This will update automatically on new commits. Configure here.

@UdeshyaDhungana UdeshyaDhungana changed the title Fix bug: ArrayElementsAssertion panics if value is an empty array Fix Bug: ArrayElementsAssertion panics if value is an empty array Nov 24, 2025
@andy1li
Copy link
Member

andy1li commented Nov 24, 2025

Thanks to @estenv for highlighting the issue!


if len(array) == 0 {
// If the specification is empty, panic
if len(a.ArrayElementAssertionSpecification) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a nil check instead? Don't think it's an array no?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, ignore me - it is an array. @UdeshyaDhungana thoughts on renaming this to ArrayElementAssertionSpecifications? (plural)

The Specification word is kinda weird too, but don't have any better ideas.

@UdeshyaDhungana UdeshyaDhungana merged commit 8b6b21c into main Nov 25, 2025
3 checks passed
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.

4 participants