Skip to content

Conversation

@tsachiherman
Copy link
Contributor

@tsachiherman tsachiherman commented May 21, 2021

Summary

This PR addresses a bug in the protocol upgrade tests where the AgreementFilterTimeout was not set correctly.
In addition, this test removes SendPaymentFromUnencryptedWallet() and replaces it with separate SendPaymentFromWallet() and GetUnencryptedWalletHandle() calls for higher efficiency.

Last, this change disables the tests parallelism. Running the upgrade tests in parallel seems to be slower overall. This is still pending investigation, but can be decoupled from these changes for now.

Test Plan

This is a test.

@tsachiherman tsachiherman self-assigned this May 21, 2021
@tsachiherman tsachiherman changed the title Improve upgrade tests testing: improve e2e protocol upgrade tests May 21, 2021
@tsachiherman tsachiherman marked this pull request as ready for review May 21, 2021 20:41
Copy link
Contributor

@algonautshant algonautshant left a comment

Choose a reason for hiding this comment

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

Looks great.

a.NoError(err)

time.Sleep(time.Second)
pongWalletHandle, err = pongClient.GetUnencryptedWalletHandle()
Copy link
Contributor

Choose a reason for hiding this comment

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

why this needs to be called at each iteration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The handle would expire after a while.. so it need to be refreshed frequently. I could have added a error-handling for that and refresh the handle only when needed, but I figured out that this would likely to be an overkill for this scenario.

@tsachiherman tsachiherman merged commit 2478653 into algorand:master May 22, 2021
@tsachiherman tsachiherman deleted the tsachi/faster_upgrade_tests branch May 22, 2021 13:00
tsachiherman added a commit to tsachiherman/go-algorand that referenced this pull request Jul 7, 2021
This PR addresses a bug in the protocol upgrade tests where the `AgreementFilterTimeout` was not set correctly.
In addition, this test removes `SendPaymentFromUnencryptedWallet()` and replaces it with separate `SendPaymentFromWallet()` and `GetUnencryptedWalletHandle()` calls for higher efficiency.

Last, this change *disables* the tests parallelism. Running the upgrade tests in parallel seems to be slower overall. This is still pending investigation, but can be decoupled from these changes for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants