fix: fix https tracing breakage in node <9 and rewrite http tests #717
Conversation
Codecov Report
@@ Coverage Diff @@
## master #717 +/- ##
==========================================
+ Coverage 90.68% 90.74% +0.06%
==========================================
Files 29 28 -1
Lines 1492 1491 -1
Branches 294 294
==========================================
Hits 1353 1353
Misses 59 59
+ Partials 80 79 -1
Continue to review full report at Codecov.
|
| @@ -183,7 +183,8 @@ function patchHttp(http: HttpModule, api: TraceAgent) { | |||
| } | |||
| } | |||
|
|
|||
| // https.get depends on https.request in <8.9 and >=8.9.1 | |||
| // https.get depends on Node http internals in 8.9.0 and 9+ instead of the | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| * because they already have a meaningful return value. | ||
| */ | ||
| class WaitForResponse { | ||
| private resolve!: (value: string) => void; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
test/plugins/test-trace-http.ts
Outdated
| // Server abstraction class definitions. These are borrowed from web framework | ||
| // tests -- which are useful because they already expose a Promise API. | ||
| const servers = { | ||
| http: Express4, https: class Express4Secure extends Express4 { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
test/plugins/test-trace-http.ts
Outdated
| // tslint:disable:no-any | ||
| this.server = this.https.createServer( | ||
| {key: Express4Secure.key, cert: Express4Secure.cert}, | ||
| this.app) as any as httpModule.Server; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
httpstracing was broken in #686 for Node 4/6/8 (except for 8.9.0). This change fixes that, and revises the http/https tracing tests (all tests are preserved except for the one measuring time specifically when calling http.get with a string argument -- I didn't think we needed that much granularity).