Skip to content

feat: add tests for filtering GET /beacon/pool/attestations #7321

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from
Open

feat: add tests for filtering GET /beacon/pool/attestations #7321

wants to merge 3 commits into from

Conversation

gap-editor
Copy link

Added test_get_beacon_pool_attestations_filtered to verify that the /eth/v2/beacon/pool/attestations endpoint correctly filters attestations based on the slot and committee_index query parameters, as required by #7312.

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

macladson
macladson requested changes Apr 13, 2025
View reviewed changes
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

Hi. Just a heads up that you have based this PR on the stable branch. Active development should be on top of unstable and target the unstable branch

@gap-editor gap-editor changed the base branch from stable to unstable April 13, 2025 11:13
@gap-editor
Copy link
Author

@macladson did i do good or no?

@michaelsproul
Copy link
Member

@gap-editor no, need to get rid of those commits from stable

@chong-he chong-he added waiting-on-author The reviewer has suggested changes and awaits thier implementation. test improvement Improve tests labels Apr 14, 2025
@gap-editor
Copy link
Author

@michaelsproul

@mergify Mergify
Copy link

mergify bot commented May 2, 2025

This pull request has merge conflicts. Could you please resolve them @gap-editor? 🙏

macladson
macladson reviewed May 3, 2025
View reviewed changes
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

Looks like there are some conflicts. Can you update your branch? (Either merge or rebase on unstable)

@gap-editor
Copy link
Author

Looks like there are some conflicts. Can you update your branch? (Either merge or rebase on unstable)

@macladson done. if smth is wrong may be better to reopen PR in unstable branch?

@gap-editor
Copy link
Author

@macladson

@jimmygchen jimmygchen mentioned this pull request May 14, 2025
Merged
@jimmygchen jimmygchen requested a review from macladson May 14, 2025 03:33
@mergify Mergify
Copy link

mergify bot commented May 15, 2025

All required checks have passed and there are no merge conflicts. This pull request may now be ready for another review.

@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 15, 2025
@mergify Mergify
Copy link

mergify bot commented Jun 5, 2025

Some required checks have failed. Could you please take a look @gap-editor? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macladson macladson Awaiting requested review from macladson

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
test improvement Improve tests waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gap-editor @CLAassistant @michaelsproul @macladson @chong-he @jimmygchen