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

change(ci): Run block proposal tests in CI #5963

Merged
merged 5 commits into from
Jan 31, 2023
Merged

change(ci): Run block proposal tests in CI #5963

merged 5 commits into from
Jan 31, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jan 16, 2023

Motivation

We want to use getblocktemplate in proposal mode in the acceptance test to check that it's producing valid templates.

Depends-On: #6027

Closes #5685

Complex Code or Requirements

We want to manually test that block proposals work before merging this automated test.

We need to fix all known block proposal and mempool bugs before running this test in CI.

Solution

  • Test all possible valid times in block proposals (min, proposal current, max, and current clamped to [min, max])
  • Run block proposal tests as part of the existing block template test in CI

Review

Anyone can review this PR.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-devops Area: Pipelines, CI/CD and Dockerfiles P-Medium ⚡ C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces labels Jan 16, 2023
@teor2345 teor2345 self-assigned this Jan 16, 2023
@github-actions github-actions bot added the C-enhancement Category: This is an improvement label Jan 16, 2023
@teor2345 teor2345 closed this Jan 16, 2023
@teor2345
Copy link
Contributor Author

That was a bad merge, let's see how this one goes

@teor2345 teor2345 reopened this Jan 16, 2023
@teor2345 teor2345 marked this pull request as ready for review January 16, 2023 01:14
@teor2345 teor2345 requested a review from a team as a code owner January 16, 2023 01:14
@teor2345 teor2345 requested review from dconnolly and removed request for a team January 16, 2023 01:14
@teor2345 teor2345 added the do-not-merge Tells Mergify not to merge this PR label Jan 16, 2023
@teor2345 teor2345 force-pushed the ci-proposal-tests branch 3 times, most recently from bb2408b to cef19bd Compare January 16, 2023 02:32
@teor2345 teor2345 force-pushed the zcash-test-block-template branch from 4d9af88 to 7dec2ff Compare January 16, 2023 03:25
@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Merging #5963 (34cba9d) into main (e20cf95) will decrease coverage by 0.16%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5963      +/-   ##
==========================================
- Coverage   78.25%   78.09%   -0.16%     
==========================================
  Files         312      312              
  Lines       39418    39418              
==========================================
- Hits        30846    30785      -61     
- Misses       8572     8633      +61     

@teor2345 teor2345 force-pushed the zcash-test-block-template branch from 7dec2ff to c96300a Compare January 17, 2023 05:08
@teor2345 teor2345 force-pushed the zcash-test-block-template branch from c96300a to ff9f6f3 Compare January 17, 2023 07:12
@teor2345 teor2345 requested review from a team as code owners January 17, 2023 07:12
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team January 17, 2023 07:12
@teor2345 teor2345 changed the base branch from main to check-mempool-lock-time January 24, 2023 22:58
@teor2345
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jan 24, 2023

update

✅ Branch has been successfully updated

@teor2345 teor2345 removed the do-not-merge Tells Mergify not to merge this PR label Jan 24, 2023
@teor2345 teor2345 requested review from arya2 and removed request for a team January 26, 2023 19:08
@teor2345
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jan 26, 2023

update

☑️ Nothing to do

  • #commits-behind>0 [:pushpin: update requirement]
  • -closed [:pushpin: update requirement]

Base automatically changed from check-mempool-lock-time to main January 27, 2023 21:46
@arya2 arya2 force-pushed the ci-proposal-tests branch from 8ab2e4c to 34cba9d Compare January 28, 2023 01:48
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

🔥

Thank you!

Update: we need another approver because I was the last pusher (it needed a rebase).

@teor2345
Copy link
Contributor Author

🔥

Thank you!

Update: we need another approver because I was the last pusher (it needed a rebase).

I can't approve because I opened the PR, so it will have to be @oxarbitrage

@teor2345
Copy link
Contributor Author

🔥
Thank you!
Update: we need another approver because I was the last pusher (it needed a rebase).

I can't approve because I opened the PR, so it will have to be @oxarbitrage or @dconnolly

If this is more annoying than it's worth, maybe we can disable this rule? Did you want to talk about it at the engineering sync?

@arya2
Copy link
Contributor

arya2 commented Jan 30, 2023

If this is more annoying than it's worth, maybe we can disable this rule? Did you want to talk about it at the engineering sync?

It doesn't happen very often so I don't mind, and tagging Pili seems like a good solution for next time. I'll ask about review-latency in the next sync though because that could use some improvement.

@teor2345
Copy link
Contributor Author

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jan 31, 2023

refresh

✅ Pull request refreshed

@teor2345
Copy link
Contributor Author

Codespell failed due to a network error, but it's not required to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-devops Area: Pipelines, CI/CD and Dockerfiles A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement C-testing Category: These are tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test the getblocktemplate RPC's response as a block template proposal in CI
3 participants