Skip to content

test: merge banning test from p2p_disconnect_ban to rpc_setban #26863

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
Closed
brunoerg wants to merge 3 commits into from
Closed

test: merge banning test from p2p_disconnect_ban to rpc_setban #26863

brunoerg wants to merge 3 commits into from

Conversation

@brunoerg
Copy link
Contributor

@brunoerg brunoerg commented Jan 10, 2023

There are tests related to banning in both p2p_disconnect_ban and rpc_setban, some of them are testing same scenarios. So, this PR moves all banning test stuff to rpc_setban and leaves p2p_disconnect_ban only for disconnecting stuff. Since p2p_disconnect_ban doesn't have any banning stuff, this PR also renames that to p2p_disconnect.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 10, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29483 (test, ci: add --v1transport option, add --v2transport to a CI task by mzumsande)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot added the Tests label Jan 10, 2023
@brunoerg
Copy link
Contributor Author

brunoerg commented Jan 10, 2023

cc: @andrewtoth (#26822 (review))

@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from 85ddb92 to c2661db Compare January 10, 2023 13:32
@brunoerg brunoerg marked this pull request as ready for review January 10, 2023 13:51
This was referenced Jan 10, 2023
Closed
Merged
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from c2661db to c248b79 Compare January 10, 2023 20:05
This was referenced Jan 10, 2023
Merged
Closed
Merged
Merged
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from c248b79 to aba0427 Compare January 17, 2023 13:18
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from aba0427 to 1ff6ed2 Compare February 7, 2023 20:35
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from 1ff6ed2 to c7397de Compare February 20, 2023 18:24
@DrahtBot DrahtBot mentioned this pull request Mar 28, 2023
Merged
@achow101 achow101 requested a review from maflcko April 25, 2023 16:28
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from c7397de to b093dd7 Compare April 27, 2023 12:47
@brunoerg
Copy link
Contributor Author

brunoerg commented Apr 27, 2023

Rebased

@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from b093dd7 to ab8be68 Compare May 1, 2023 22:28
@brunoerg
Copy link
Contributor Author

brunoerg commented May 1, 2023

Rebased

@DrahtBot DrahtBot removed the CI failed label May 2, 2023
This was referenced Sep 10, 2023
Merged
Merged
@DrahtBot DrahtBot mentioned this pull request Sep 20, 2023
Merged
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from 35b9e59 to 8bdfbcf Compare October 3, 2023 13:29
@DrahtBot DrahtBot mentioned this pull request Feb 3, 2024
Merged
@brunoerg
test: merge setban test from p2p_disconnect_ban to rpc_setban
54d0e32 
Sometimes both ones are testing same stuff related to banning, it makes sense to merge them to avoid repetition.
@brunoerg
test: remove ban test from p2p_disconnect_ban
370b843
@brunoerg
rename p2p_disconnect_ban to p2p_disconnect
682ba54
@brunoerg brunoerg force-pushed the 2023-01-merge-ban-test branch from 8bdfbcf to 682ba54 Compare February 13, 2024 21:46
@brunoerg
Copy link
Contributor Author

brunoerg commented Feb 13, 2024

Rebased

@glozow
Copy link
Member

glozow commented Feb 16, 2024

Since there doesn't seem to be very much review interest... is there any benefit in moving these tests to a different file?

@brunoerg
Copy link
Contributor Author

brunoerg commented Feb 16, 2024

Since there doesn't seem to be very much review interest... is there any benefit in moving these tests to a different file?

Currently, there are stuff that are being test in both p2p_disconnect_ban and rpc_setban (e.g. "Test the ban list is preserved through restart" in setban and "setban: test persistence across node restart" in p2p_disconnect_ban). Since we have a specific test for setban RPC, the idea is concentrate everything about it there.

@DrahtBot DrahtBot mentioned this pull request Feb 26, 2024
Merged
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 11, 2024

🐙 This pull request conflicts with the target branch and needs rebase.

@brunoerg
Copy link
Contributor Author

brunoerg commented Mar 20, 2024

I'll close for now and work in a simpler alternative.

@brunoerg brunoerg closed this Mar 20, 2024
@brunoerg brunoerg mentioned this pull request Apr 22, 2024
Merged
@bitcoin bitcoin locked and limited conversation to collaborators Mar 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@maflcko maflcko Awaiting requested review from maflcko

Assignees

No one assigned

Labels

Needs rebase Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@brunoerg @DrahtBot @glozow