Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reactivate indexer tests #1414

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

eugypalu
Copy link
Contributor

@eugypalu eugypalu commented Sep 27, 2024

Resolves: #

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing

What is the new behavior?

reactivate indexer tests

Does this introduce a breaking change?

  • Yes
  • No

@tcoratger
Copy link
Collaborator

Lgtm, pending @greged93. In a follow up PR, could we make a test util function that allows to generate a random starknet calldata for example with a comprehensible implementation of each element of the array?

Currently, I see that the test passes but I am not really able to understand how the calldata was built and if it is correct or not.

I would say for example that we need a function where we enter parameters (like the to, etc) and the function returns us the clean and well documented calldata so that anyone is able to generate a test calldata even first time contributors.

@greged93
Copy link
Contributor

let's maybe make an issue for this

@tcoratger
Copy link
Collaborator

tcoratger commented Sep 30, 2024

let's maybe make an issue for this

Done #1417

@tcoratger
Copy link
Collaborator

@greged93 Can we merge or is there a blocker due to your current integration work?

@greged93
Copy link
Contributor

greged93 commented Oct 2, 2024

sure let me quickly review and we can merge!

@greged93 greged93 merged commit 6ef788d into kkrt-labs:main Oct 2, 2024
7 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.

3 participants