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

[FAB-17192] Add unit tests for executeCommitter and commitUpdates #621

Merged
merged 1 commit into from Feb 7, 2020

Conversation

DereckLuo
Copy link
Contributor

Signed-off-by: Chongxin Luo Chongxin.Luo@ibm.com

Type of change

  • Test update

Description

  • Added statecouchdb unit test for executeCommitter and commitUpdates functions.

Additional details

Related issues

FAB-17192

@DereckLuo DereckLuo requested a review from a team as a code owner February 3, 2020 14:39
@DereckLuo DereckLuo force-pushed the FAB-17192 branch 2 times, most recently from bc37567 to 4efe7fb Compare February 5, 2020 02:15

err = db.executeCommitter(committers)
assert.NoError(t, err)

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a check for key/values finally committed in the db

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manish-sethi check has been added, thanks.

assert.EqualError(t, err, "error handling CouchDB request. Error:bad_request, Status Code:400, Reason:`docs` parameter must be an array.")
}

func TestCommitUpdates(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the tasks of commitUpdate() function is to update the revision present in the cache. This is done by maintaining the newly created revision in the cacheKVs map present in the committer object. It would be good to add a test to check whether the revision is being updated correctly. We can do the following to check that:

  1. add the key and the empty revision (as the key is new) to the cacheKVs map in the committer object.
  2. once the committer completes its execution, it would update the cacheKVs with the assigned revision for the key.
  3. check whether the revision has been changed in the cacheKVs map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the review. new test cases has been added.

}
err = committer.commitUpdates()
assert.NoError(t, err)
assert.NotEmpty(t, committer.cacheKVs["key2"])
Copy link
Contributor

Choose a reason for hiding this comment

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

One minor change is required but the rest looks good to me.
assert.NotEmpty(t, committer.cacheKVs["key2"]) would always be true as per line 294. The main thing that changes after running committer.commitUpdates() is the cacheKVs["key2"].AdditionalInfo which holds the revision.

Replace line 299 with assert.NotEmpty(t, committer.cacheKVs["key2"].AdditionalInfo) Here, cacheKVs["key2"].AdditionalInfo would be non-empty after running committer.commitUpdates() but empty before that.

- Added statecouchdb unit test for executeCommitter and commitUpdates functions.

Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com>
@manish-sethi
Copy link
Contributor

Thanks @DereckLuo and @cendhu for additional review.

@manish-sethi manish-sethi merged commit 8ccf64e into hyperledger:master Feb 7, 2020
@DereckLuo DereckLuo deleted the FAB-17192 branch February 27, 2020 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants