cairo-contracts icon indicating copy to clipboard operation
cairo-contracts copied to clipboard

feat(testing): verify indexed keys in event assertions

Open turkaturki opened this issue 9 months ago • 7 comments

Fixes #1054

Add verification of indexed keys in event assertions to ensure proper event structure testing. This change:

  • Adds verify_indexed_keys() function to check event key matching
  • Implements key verification in is_emitted() alongside data matching
  • Updates EventSpyQueue to handle ordered event assertions

PR Checklist

  • [x] Tests
  • [x] Added entry to CHANGELOG.md

turkaturki avatar Apr 03 '25 18:04 turkaturki

Hey @ericnordelo , could you please review this PR when you get a chance?

turkaturki avatar Apr 03 '25 18:04 turkaturki

Hi @ericnordelo , just following up on this. Would appreciate it if you could take a look at the PR when you have some time. Happy to address any feedback. Thanks!

turkaturki avatar Apr 05 '25 11:04 turkaturki

Hey. Thanks again for taking the time. I will review it it early this week. Thanks for your patience!

ericnordelo avatar Apr 05 '25 11:04 ericnordelo

Thanks for the feedback! @ericnordelo I understand your point about that approach potentially being redundant. I've updated the implementation and changed the approach based on your comment. Could you please take a look and let me know if I'm heading in the right direction or if there’s anything else I should adjust?

turkaturki avatar Apr 10 '25 12:04 turkaturki

Thanks for the clarification! I've reverted is_emitted to its original form.

turkaturki avatar Apr 14 '25 17:04 turkaturki

Hi @ericnordelo, just following up on this PR. I’ve implemented the suggested changes as discussed. Could you please take another look when you have a moment? Let me know if anything else needs adjustment. Thanks again!

turkaturki avatar Apr 22 '25 16:04 turkaturki

@ericnordelo Could you kindly check the modified changes.

turkaturki avatar Jun 03 '25 20:06 turkaturki

Hey @turkaturki, thanks for the patience! Note that we still have a couple of issues with the PR, including some tests failing and adding keys to events that don't require them. To be mindful of your time and avoid delaying this feature, I will take this PR and complete it to get it merged as soon as possible, including, of course, your commits. Thanks again for the contribution!

ericnordelo avatar Jul 08 '25 11:07 ericnordelo

Closing in favor of #1472

ericnordelo avatar Jul 28 '25 12:07 ericnordelo