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

Khalil/1662 epoch test suite updates #2041

Merged
merged 4 commits into from Feb 23, 2022

Conversation

kc1116
Copy link
Contributor

@kc1116 kc1116 commented Feb 22, 2022

This PR adds some small updates to the epoch integration test suite:

  • Make sure we don't pause the container we connect to with the flow client in the AN suite
  • Increase the wait time for WaitForSealedView from 2 min -> 3min to decrease flakiness in SN suite

- avoid pausing access node used for client connections when running the access node suite
Comment on lines 212 to 219
if role == flow.RoleAccess {
containerToReplace = s.getContainerToReplace(role, "access_2")
require.NotNil(s.T(), containerToReplace)
} else {
// grab the first container of this node role type, this is the container we will replace
containerToReplace = s.getContainerToReplace(role, "")
require.NotNil(s.T(), containerToReplace)
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it be more clear to have two functions:

Suggested change
if role == flow.RoleAccess {
containerToReplace = s.getContainerToReplace(role, "access_2")
require.NotNil(s.T(), containerToReplace)
} else {
// grab the first container of this node role type, this is the container we will replace
containerToReplace = s.getContainerToReplace(role, "")
require.NotNil(s.T(), containerToReplace)
}
if role == flow.RoleAccess {
containerToReplace = s.getContainerToReplaceByName("access_2")
require.NotNil(s.T(), containerToReplace)
} else {
// grab the first container of this node role type, this is the container we will replace
containerToReplace = s.getContainerToReplaceByRole(role)
require.NotNil(s.T(), containerToReplace)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kc1116 kc1116 merged commit 18a197a into master Feb 23, 2022
@kc1116 kc1116 deleted the khalil/1662-epoch-test-suite-updates branch February 23, 2022 21:20
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