-
Notifications
You must be signed in to change notification settings - Fork 582
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
ci: Run tests using PostgreSQL database and mock #49
Conversation
This allows us to use the mock database for quick iterative testing, and have confidence from CI using a real PostgreSQL database. PostgreSQL tests are only ran on Linux. They are *really* slow on MacOS and Windows runners, and don't provide much additional confidence.
Codecov Report
@@ Coverage Diff @@
## main #49 +/- ##
==========================================
- Coverage 71.31% 71.12% -0.20%
==========================================
Files 59 59
Lines 2308 2327 +19
Branches 30 30
==========================================
+ Hits 1646 1655 +9
- Misses 517 525 +8
- Partials 145 147 +2
Continue to review full report at Codecov.
|
- name: Test with PostgreSQL Database | ||
if: runner.os == 'Linux' | ||
run: | ||
DB=true gotestsum --jsonfile="gotests.json" --packages="./..." -- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! It's neat that it's so easy to test both ways.
I like that we get the best of both worlds here - the mock DB seems ideal for quick iteration because it's so fast, but testing against a real PostgreSL driver is important too, but doesn't have to slow down test authoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed!
@@ -86,13 +86,14 @@ INSERT INTO | |||
email, | |||
name, | |||
login_type, | |||
revoked, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem related to the ability to switch between DB / mock - but maybe needed to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this was actually a bug the DB implementation caught!
Not all resources were cleaned up immediately after a peer connection was closed. DataChannels could have a goroutine exit after Close() prior to this.
@@ -135,6 +135,7 @@ func (c *Channel) init() { | |||
|
|||
c.conn.dcDisconnectListeners.Add(1) | |||
c.conn.dcFailedListeners.Add(1) | |||
c.conn.dcClosedWaitGroup.Add(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch on these! Looks like a candidate for a backport to cdr/m
too - seems like we're missing this wait logic there too
This allows us to use the mock database for quick iterative testing,
and have confidence from CI using a real PostgreSQL database.
PostgreSQL tests are only ran on Linux. They are really slow on MacOS
and Windows runners, and don't provide much additional confidence.
This also allows
DB=true go test ...
locally to leverage our Docker PostgreSQLcode for running tests!