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

Test/test execution execution.go #96

Closed

Conversation

18aaddy
Copy link

@18aaddy 18aaddy commented Oct 29, 2024

This addresses #95

I have written tests for all functions in execution/execution.go except GetAccount() and GetTransactionReceipt() as they depend on state and need to have valid proofs in order to work.
I have made some modifications to execution.go which solved some errors.

This PR depends on my earlier PR #90 as proof.go had been updated in that. It should be merged after PR #90

@18aaddy 18aaddy mentioned this pull request Oct 29, 2024
2 tasks
@18aaddy
Copy link
Author

18aaddy commented Oct 29, 2024

Here, the test checks are failing because the CreateAccessList() function is not yet working correctly, which I have mentioned in issue #89

The lint is failing because of a for loop in the test file but the loop is needed as it waits for the blocks to be pushed into state before running tests. Without the loop, the test fails. Also, locally it is only a warning, not an error

@gerceboss
Copy link
Contributor

@18aaddy , make changes according to the one that @x-senpai-x writes

@18aaddy
Copy link
Author

18aaddy commented Oct 30, 2024

You mean the types.AccessList PR? #94

@18aaddy
Copy link
Author

18aaddy commented Oct 30, 2024

@gerceboss This PR contains all tests for http_rpc and execution.go and all tests are passing. This can be merged with no need to merge #88 or #90.
I tried to split the PRs but either the tests aren't passing or there is a merge conflict.

Then @ABD-AZE can work on CreateAccessList function.

The lint is failing because of a for loop, and I tried to replace that but the tests were failing. It is only a warning while running locally.

ABD-AZE added a commit to ABD-AZE/selene that referenced this pull request Nov 1, 2024
@ABD-AZE ABD-AZE mentioned this pull request Nov 1, 2024
@18aaddy
Copy link
Author

18aaddy commented Nov 1, 2024

I am closing this PR. PR #100 can be used instead

@18aaddy 18aaddy closed this Nov 1, 2024
ABD-AZE added a commit to ABD-AZE/selene that referenced this pull request Nov 4, 2024
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.

2 participants