-
Notifications
You must be signed in to change notification settings - Fork 51
Conversation
Codecov Report
@@ Coverage Diff @@
## master #663 +/- ##
======================================
Coverage 88.9% 88.9%
======================================
Files 15 15
Lines 946 946
Branches 83 83
======================================
Hits 841 841
Misses 87 87
Partials 18 18 Continue to review full report at Codecov.
|
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 LGTM, and is better than nothing -- is there any way for us to detect that logs have been emitted to stackdriver?
|
||
it('should run the quickstart', done => { | ||
// select a random port between 49152 and 65535 | ||
const PORT = Math.floor((Math.random() * (65535-49152))) + 49152; |
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.
just a difference in style, I'd usually just pick a port and use it consistently for integration tests (I'd rather know that I'm not shutting down cleanly via failure to bind).
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.
In retrospect, that's probably going to be fine too. Initially I was worried about the same tests running in parallel, but it doesn't matter since they're almost always in docker containers that should abstract it from us.
@DominicKramer from @bcoe question:
|
@bcoe can you elaborate.. as far as I can see there are no logs being emitted to a Stackdriver log stream. I only see |
@ofrobots I think I'm misunderstanding the default behavior of |
There are currently no tests available for the samples. This cleans up a few things and adds a simple test. In addition removes
@google-cloud/repo-tools
and it's configuration as it wasn't being used.