Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make uninitializedBegin test accurately test its intention #3244

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

mhutchinson
Copy link
Contributor

This was previously creating and initializing a tree, and then testing what happened if you created a transaction on a tree ID that definitely didn't exist. What it was trying to test was something different, which is the case where a tree had been created/defined, but was not initialized with an empty log root yet. The test now reflects that. This allows #3201 to avoid a nil check for something that otherwise will be guaranteed to exist.

This was previously creating and initializing a tree, and then testing what happened if you created a transaction on a tree ID that definitely didn't exist. What it was trying to test was something different, which is the case where a tree had been created/defined, but was not initialized with an empty log root yet. The test now reflects that. This allows google#3201 to avoid a nil check for something that otherwise will be guaranteed to exist.
@mhutchinson mhutchinson requested a review from a team as a code owner December 11, 2023 12:20
@mhutchinson mhutchinson requested a review from AlCutter December 11, 2023 12:20
@mhutchinson mhutchinson merged commit 679237e into google:master Dec 11, 2023
@mhutchinson mhutchinson deleted the betterNotInitTest branch December 11, 2023 12:36
mhutchinson added a commit to mhutchinson/trillian that referenced this pull request Dec 11, 2023
Test was updated in google#3244 to test the correct behaviour, which means that we don't need to worry here about whether the tree exists or not.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants