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.

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

test: ProcessProposal with invalid namespace in index wrapped PFB tx #1614

Merged
merged 11 commits into from
Apr 13, 2023
Merged

test: ProcessProposal with invalid namespace in index wrapped PFB tx #1614

merged 11 commits into from
Apr 13, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Apr 11, 2023

Closes #1558

@rootulp rootulp added the testing items that are strictly related to adding or extending test coverage label Apr 11, 2023
@rootulp rootulp added this to the Mainnet milestone Apr 11, 2023
@rootulp rootulp self-assigned this Apr 11, 2023
@rootulp rootulp marked this pull request as ready for review April 11, 2023 19:42
@MSevey MSevey requested review from a team and rach-id and removed request for a team April 11, 2023 21:26
rach-id
rach-id reviewed Apr 11, 2023
View reviewed changes
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Would defer to other reviewers for final approval

app/test/process_proposal_test.go Show resolved Hide resolved
evan-forbes
evan-forbes reviewed Apr 12, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

one blocking question, otherwise LGTM

testutil/blobfactory/payforblob_factory.go Outdated Show resolved Hide resolved
testutil/blobfactory/payforblob_factory.go Show resolved Hide resolved
testutil/blobfactory/payforblob_factory.go Outdated Show resolved Hide resolved
@MSevey MSevey requested a review from a team April 12, 2023 20:36
@rootulp
Copy link
Collaborator Author

rootulp commented Apr 12, 2023

@sweexordious FWIW I'll need two approvals to merge this so if you don't feel comfortable explicitly approving, I can add another reviewer.

@codecov-commenter
Copy link

Codecov Report

Merging #1614 (ae01b98) into main (e905143) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1614   +/-   ##
=======================================
  Coverage   49.17%   49.17%           
=======================================
  Files          84       84           
  Lines        4948     4948           
=======================================
  Hits         2433     2433           
  Misses       2277     2277           
  Partials      238      238

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rach-id
rach-id approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

Oh no sorry, i didnt know that was activated. Thanks for pinign

@rootulp rootulp merged commit dd19531 into celestiaorg:main Apr 13, 2023
24 checks passed
@rootulp rootulp deleted the rp/process-proposal-test branch April 13, 2023 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rach-id rach-id rach-id approved these changes

@evan-forbes evan-forbes evan-forbes approved these changes

Assignees

@rootulp rootulp

Labels
testing items that are strictly related to adding or extending test coverage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create ProcessProposal test with invalid namespace
4 participants
@rootulp @codecov-commenter @rach-id @evan-forbes