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

Add tests for Commiter change #5

Merged
merged 1 commit into from
Feb 3, 2023
Merged

Add tests for Commiter change #5

merged 1 commit into from
Feb 3, 2023

Conversation

prettymuchbryce
Copy link

@prettymuchbryce prettymuchbryce commented Feb 3, 2023 •
edited
Loading

The Commiter change here was not unit tested. This PR adds unit tests for this change.

@prettymuchbryce prettymuchbryce changed the title Add tests for CommitBlocker change Add tests for Commiter change Feb 3, 2023
@prettymuchbryce prettymuchbryce force-pushed the prettymuchbryce/add-additional-commiter-tests branch from bd28a85 to 7acbfab Compare February 3, 2023 15:01
jayy04
jayy04 reviewed Feb 3, 2023
View reviewed changes
baseapp/abci_test.go
wasCommiterCalled = true
}

app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
Copy link

@jayy04 jayy04 Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

is this needed to reset/initialize deliverState?

Copy link
Author

Choose a reason for hiding this comment

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

I wondered the same thing, but the test fails with a nil pointer deference here when Commit tries to reference deliverState.ctx, which is nil (it's set by BeginBlock using the block header here).

@prettymuchbryce prettymuchbryce merged commit bc3d819 into dydx-fork-v0.47.0-alpha2 Feb 3, 2023
@prettymuchbryce prettymuchbryce deleted the prettymuchbryce/add-additional-commiter-tests branch February 3, 2023 16:57
prettymuchbryce added a commit that referenced this pull request Feb 28, 2023
@prettymuchbryce
Add tests for Commiter change (#5)
ad8dc88
prettymuchbryce added a commit that referenced this pull request Feb 28, 2023
@prettymuchbryce
Add tests for Commiter change (#5)
5d6053c
prettymuchbryce added a commit that referenced this pull request Mar 22, 2023
@prettymuchbryce
Add tests for Commiter change (#5)
862b4d8
jonfung-dydx pushed a commit that referenced this pull request Mar 23, 2023
@prettymuchbryce @jonfung-dydx
Add tests for Commiter change (#5)
2a47005
jonfung-dydx pushed a commit that referenced this pull request Mar 23, 2023
@prettymuchbryce @jonfung-dydx
Add tests for Commiter change (#5)
93d2285
jonfung-dydx added a commit that referenced this pull request Mar 24, 2023
@jonfung-dydx @prettymuchbryce @dydxwill
Add dYdX commits to cosmos v0.47.1 (#11)
0c25c44 
* Implement Commiter

* Update README with fork instructions

* Enable CI workflows and fix failures (#4)

* Fix lint/gocritic error

* Disable some workflow jobs

* Add tests for Commiter change (#5)

* Add additional upgrade step to README

* add precommit callback (#7)

* Revert "add precommit callback (#7)" (#10)

This reverts commit 0e85c49.

* cosmos-sdk upgrade README changes

---------

Co-authored-by: Bryce Neal <bryce@dydx.exchange>
Co-authored-by: Bryce Neal <brycedneal@gmail.com>
Co-authored-by: dydxwill <119354122+dydxwill@users.noreply.github.com>
jonfung-dydx pushed a commit that referenced this pull request Mar 24, 2023
@prettymuchbryce @jonfung-dydx
Add tests for Commiter change (#5)
b81140e
jonfung-dydx pushed a commit that referenced this pull request Mar 28, 2023
@prettymuchbryce @jonfung-dydx
Add tests for Commiter change (#5)
606475c
jonfung-dydx pushed a commit that referenced this pull request Mar 28, 2023
@prettymuchbryce @jonfung-dydx
Add tests for Commiter change (#5)
7bfb867
roy-dydx pushed a commit that referenced this pull request Jun 30, 2023
@prettymuchbryce @roy-dydx
Add tests for Commiter change (#5)
e1463c8
roy-dydx pushed a commit that referenced this pull request Jul 1, 2023
@prettymuchbryce @roy-dydx
Add tests for Commiter change (#5)
066c64b
roy-dydx pushed a commit that referenced this pull request Jul 2, 2023
@prettymuchbryce @roy-dydx
Add tests for Commiter change (#5)
a218d05
roy-dydx pushed a commit that referenced this pull request Jul 3, 2023
@prettymuchbryce @roy-dydx
Add tests for Commiter change (#5)
6e7a25b
lcwik pushed a commit that referenced this pull request Aug 24, 2023
@prettymuchbryce @lcwik
Add tests for Commiter change (#5)
3366bdc
lcwik added a commit that referenced this pull request Feb 26, 2024
@lcwik
# This is a combination of 5 commits.
064329f 
# This is the 1st commit message:

Enable locking kv store

# The commit message #2 will be skipped:

# Fix some lock orderings

# The commit message #3 will be skipped:

# Fix minor typo

# The commit message #4 will be skipped:

# Ensure that writes happen in a deterministic order.
#
# Ensure that reads are also done all the time, remove this if it doesn't impact gas.

# The commit message #5 will be skipped:

# Remove locking for now for lockingkv.Get/Has
lcwik added a commit that referenced this pull request Feb 26, 2024
@lcwik
# This is a combination of 5 commits.
3fe1d6c 
# This is the 1st commit message:

Enable locking kv store

# The commit message #2 will be skipped:

# Fix some lock orderings

# The commit message #3 will be skipped:

# Fix minor typo

# The commit message #4 will be skipped:

# Ensure that writes happen in a deterministic order.
#
# Ensure that reads are also done all the time, remove this if it doesn't impact gas.

# The commit message #5 will be skipped:

# Remove locking for now for lockingkv.Get/Has
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jayy04 jayy04 jayy04 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@prettymuchbryce @jayy04