Skip to content
This repository was archived by the owner on Jan 21, 2026. It is now read-only.

fix: fix https tracing breakage in node <9 and rewrite http tests#717

Merged
kjin merged 3 commits intogoogleapis:masterfrom
kjin:https-2.7
Apr 5, 2018
Merged

fix: fix https tracing breakage in node <9 and rewrite http tests#717
kjin merged 3 commits intogoogleapis:masterfrom
kjin:https-2.7

Conversation

@kjin
Copy link
Copy Markdown
Contributor

@kjin kjin commented Apr 4, 2018

https tracing 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).

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 4, 2018
@kjin kjin changed the title [WIP] fix: fix https tracing breakage in node <9 fix: fix https tracing breakage in node <9 and rewrite tests Apr 4, 2018
@kjin kjin changed the title fix: fix https tracing breakage in node <9 and rewrite tests fix: fix https tracing breakage in node <9 and rewrite http tests Apr 4, 2018
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 4, 2018

Codecov Report

Merging #717 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
src/config.ts 100% <ø> (ø) ⬆️
src/plugins/plugin-http.ts 91.56% <100%> (+1.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c66b041...d3cf86b. Read the comment docs.

}

// 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.

* 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.

This comment was marked as spam.

Comment thread 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.

Comment thread 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.

@kjin kjin merged commit a3ea16d into googleapis:master Apr 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants