Conversation
37adec2 to
6e1d14f
Compare
7daa2a8 to
175e22f
Compare
b43d6db to
0b5e99b
Compare
|
|
||
| // Save the auth object to the client, for use by other methods. | ||
| this.auth = this._gaxGrpc.auth as GoogleAuth; | ||
| this.auth = gaxGrpc.auth as GoogleAuth; |
There was a problem hiding this comment.
Why are you removing gaxGrpc from the instantiated class?
There was a problem hiding this comment.
This is one of the reasons why the existing code does not work in browser with webpack.
To ask the client class to be browser-compatible and use fallback.ts implementation, we pass fallback: 'proto' or fallback: 'rest' to the constructor, so that the client won't use gRPC (which does not work in browser) and use proto-over-HTTP or REGAPIC (JSON-over-HTTP) instead.
const client = new library.WhateverClient({fallback: 'proto'}); // fallback: true also worksThis affects how the client accesses gax (whether it would choose gax or gax.fallback), and what it will assign to its gaxGrpc, e.g. here is this place in Speech API.
Now, the Location service is a "mixin", basically, it's a separate client class that can be attached to the client that exposes Location API (that's why it lives in the common place, in google-gax).
If Speech client used fallback and created gaxGrpc from the fallback implementation, it must use the same gaxGrpc for all other clients (Location and LRO). For that to happen, the constructors for Location client and Operations client accept the existing instance of gaxGrpc (line 101 of this file). But I found that the instance that was passed is not actually used in some of these places, which makes the fallback client instantiate Location client that uses gRPC, so I'm changing all these places to use the instance that was passed down.
| ); | ||
| }; | ||
| } | ||
| if ( |
There was a problem hiding this comment.
I don't see these errors being tested explicitly.
There was a problem hiding this comment.
The browser use case tests it in test/browser-test/test.endtoend.ts, lines 86-92.
It seems the REGAPIC use case (the first throw) is indeed not tested, I added tests for all these cases to the test application (test/test-application/src/index.ts, functions testExpandThrows, testCollectThrows, testChatThrows). Thank you for noticing it!
717e40c to
ff0598a
Compare
Fixes googleapis/google-cloud-node-core#309.
This is a big refactor of the tests for google-gax that is long overdue.
I'm splitting system tests and browser tests into a separate subpackages (monorepo-style), each of which depend on the prepacked tarball of
google-gaxand other dependencies, simulating real life usage (so nonpm linkor anything like that, which confuses TypeScript compiler; just the real tarballs copied everywhere, sonpm installworks just as it would work in the real life).Subpackages created in
test/:showcase-server: extracted from system tests; a helper to download and rungapic-showcasebinary;showcase-echo-client: a client library generated bygapic-generator-typescriptbased on Showcase protos that we will use for testinggoogle-gaxin the next two subpackages:test-application: an application that launchesshowcase-serverand usesshowcase-echo-clientto connect to it and test variousgoogle-gaxfeatures;browser-test: a test suite that launcheskarmaand runs some tests in a real Chromium browser.This PR also contains some code fixes that will be submitted separately as #1311.