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

fix(deps): Add test dependency from zebra-rpc to zebra-network with correct features #5992

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

teor2345
Copy link
Contributor

Motivation

If I build the tests for the individual zebra-rpc crate, the build fails due to a missing dependency.

This is a bug from PR #5951.

Solution

Add the correct dependencies for the proptest-impl feature.

Review

Anyone can review this code. It blocks some kinds of testing, so I think it might be a high priority.

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?

Follow Up Work

I don't think we need any extra CI checks for this build bug, because it just happens in test code.

@teor2345 teor2345 added C-bug Category: This is a bug A-dependencies Area: Dependency file updates P-High 🔥 I-build-fail Zebra fails to build C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Jan 18, 2023
@teor2345 teor2345 self-assigned this Jan 18, 2023
@teor2345 teor2345 requested a review from a team as a code owner January 18, 2023 03:34
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team January 18, 2023 03:34
@github-actions github-actions bot added the C-feature Category: New features label Jan 18, 2023
@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #5992 (d41f9ee) into main (6a06cbf) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5992      +/-   ##
==========================================
- Coverage   78.03%   78.02%   -0.01%     
==========================================
  Files         312      312              
  Lines       38995    38995              
==========================================
- Hits        30430    30427       -3     
- Misses       8565     8568       +3     

@oxarbitrage
Copy link
Contributor

Failed with:

ERROR: failed to solve: failed to push us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:pr-5992: failed to copy: io: read/write on closed pipe
Error: buildx failed with: ERROR: failed to solve: failed to push us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:pr-5992: failed to copy: io: read/write on closed pipe

Iam restarting the failed check.

Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

looks good, if everything pass it should be ok to merge.

@teor2345
Copy link
Contributor Author

Failed with:

ERROR: failed to solve: failed to push us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:pr-5992: failed to copy: io: read/write on closed pipe
Error: buildx failed with: ERROR: failed to solve: failed to push us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:pr-5992: failed to copy: io: read/write on closed pipe

Iam restarting the failed check.

PR #5985 fixes this issue, it might not have merged when I pushed this branch.

@teor2345
Copy link
Contributor Author

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jan 19, 2023

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request Jan 19, 2023
@teor2345
Copy link
Contributor Author

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jan 19, 2023

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request Jan 19, 2023
@teor2345
Copy link
Contributor Author

@Mergifyio refresh

@teor2345
Copy link
Contributor Author

Hopefully this should merge now that we've fixed that CI Docker bug

@mergify
Copy link
Contributor

mergify bot commented Jan 19, 2023

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request Jan 19, 2023
@mergify mergify bot merged commit 67194c4 into main Jan 19, 2023
@mergify mergify bot deleted the fix-build-dependency branch January 19, 2023 09:12
@oxarbitrage oxarbitrage mentioned this pull request Jan 29, 2023
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates C-bug Category: This is a bug C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG I-build-fail Zebra fails to build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants