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

Travis - Remove the t.parallel() from TestChannelLinkBidirectionalOneHopePayments. #1589

Closed
wants to merge 1 commit into from
Closed

Travis - Remove the t.parallel() from TestChannelLinkBidirectionalOneHopePayments. #1589

wants to merge 1 commit into from

Conversation

offerm
Copy link
Contributor

@offerm offerm commented Jul 19, 2018

This test is CPU intensive and creates ~966 goroutings. While the test itself is OK it has impact on other tests that are running in parallel. In these other tests we can find sleep(time) that is used for sync. Due to this test the sleep(time) may be too short and may cause these tests to fail.

…nts.

This test is CPU intensive and creates ~966 goroutings. While the test itself is OK it has impact on other tests that are running in parallel. In these other tests we can find sleep(time) that is used for sync. Due to this test the sleep(time) may be too short and may cause these tests to fail.
@offerm offerm changed the title Remove the t.parallel() from TestChannelLinkBidirectionalOneHopePayments. Travis - Remove the t.parallel() from TestChannelLinkBidirectionalOneHopePayments. Jul 19, 2018
@Roasbeef
Copy link
Member

Roasbeef commented Aug 9, 2018

It shouldn't be an issue that this test is running, instead the other tests in the package may need to have their timing assumptions checked to ensure that they can execute in parallel with other tests that use a high number of goutines.

@Roasbeef Roasbeef closed this Aug 9, 2018
@offerm
Copy link
Contributor Author

offerm commented Aug 10, 2018

@Roasbeef Agree in general.

Still, since the impact of this test is random and the developer may see any of the other tests failing without a good reason, I suggest removing the parallel attribute from this test until

"the other tests in the package may need to have their timing assumptions checked to ensure that they can execute in parallel with other tests that use a high number of goutines."

Keep in mind that external contributors can't "restart" the Travis process upon such random failure and are basically blocked not knowing if what they did is actually good and not knowing if the PR (that failed the test) will be reviewed by the team.

Consider this as a temporary fix aiming to enhance productivity for the community.

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.

2 participants