Move integration tests' db initialization to BeforeSuite#76
Merged
Move integration tests' db initialization to BeforeSuite#76
Conversation
c02fe4a to
53b6668
Compare
53b6668 to
3196a59
Compare
rmulhol
approved these changes
Dec 17, 2019
Contributor
rmulhol
left a comment
There was a problem hiding this comment.
👍 to reducing duplication and improving performance 🎉
imo totally worth the red squiggles - especially given that we're hitting that too many connection error.
Would propose putting a story on the board to figure out what's going on with GoLand - there's gotta be a way we can fix that.
Contributor
Author
|
Update incoming that fixes red squiggles by renaming the integration test files to end with a |
yaoandrew
approved these changes
Dec 17, 2019
Contributor
yaoandrew
left a comment
There was a problem hiding this comment.
Cool.. yeah I think that test file convention in go is what goland is leaning on. I bet this fixes a bunch of things like the ability to run tests for the package from within each file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is just a proof of concept applied to a couple integration test files-- wanted to get eyes on it before proceeding further.This PR fixes a "too many connections" error when we run our integration tests, and I think it's a solid change for reducing duplication and improving performance as well. The main issue I'm encountering is that GoLand doesn't seem to know
dbandblockChain(fromintegration_tests_suite_test.go) are in scope for the integration tests, which is weird because those vars are being declared in the same package.Interested to hear people's thoughts and suggestions.
If this issue is solvable or acceptable, I'll move on with this approach. Otherwise we'll have to find a different way of solving the "too many connections" error.For the time being, I think this approach is the best path forward, although the red squiggles are definitely annoying.