-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat(deps): template change for new gts
dependency
#354
Changes from 4 commits
53837ec
296b892
ac56bdd
9654177
5cca47f
43c8654
3045ec8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
import * as protosTypes from '../protos/protos'; | ||
import * as assert from 'assert'; | ||
import { describe, it } from 'mocha'; | ||
/* eslint-disable @typescript-eslint/no-var-requires */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated, but I would love to have a proper There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, Justin also brought this up in the other PR, import module will bring types in. Basically it's about mock methods like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you spend an hour (but not more) on that? If we can make A better alternative is to move unit tests to |
||
const bigquerystorageModule = require('../src'); | ||
|
||
import {PassThrough} from 'stream'; | ||
|
@@ -41,7 +42,9 @@ export interface Callback { | |
} | ||
|
||
export class Operation{ | ||
/* eslint-disable @typescript-eslint/no-empty-function */ | ||
constructor(){}; | ||
/* eslint-disable @typescript-eslint/no-empty-function */ | ||
promise() {}; | ||
} | ||
function mockSimpleGrpcMethod(expectedRequest: {}, response: {} | null, error: FakeError | null) { | ||
|
@@ -155,8 +158,6 @@ describe('v1beta1.BigQueryStorageClient', () => { | |
request.tableReference.projectId = ''; | ||
request.tableReference = {}; | ||
request.tableReference.datasetId = ''; | ||
// Mock response | ||
const expectedResponse = {}; | ||
// Mock gRPC layer | ||
client._innerApiCalls.createReadSession = mockSimpleGrpcMethod( | ||
request, | ||
|
@@ -209,8 +210,6 @@ describe('v1beta1.BigQueryStorageClient', () => { | |
const request: protosTypes.google.cloud.bigquery.storage.v1beta1.IBatchCreateReadSessionStreamsRequest = {}; | ||
request.session = {}; | ||
request.session.name = ''; | ||
// Mock response | ||
const expectedResponse = {}; | ||
// Mock gRPC layer | ||
client._innerApiCalls.batchCreateReadSessionStreams = mockSimpleGrpcMethod( | ||
request, | ||
|
@@ -263,8 +262,6 @@ describe('v1beta1.BigQueryStorageClient', () => { | |
const request: protosTypes.google.cloud.bigquery.storage.v1beta1.IFinalizeStreamRequest = {}; | ||
request.stream = {}; | ||
request.stream.name = ''; | ||
// Mock response | ||
const expectedResponse = {}; | ||
// Mock gRPC layer | ||
client._innerApiCalls.finalizeStream = mockSimpleGrpcMethod( | ||
request, | ||
|
@@ -317,8 +314,6 @@ describe('v1beta1.BigQueryStorageClient', () => { | |
const request: protosTypes.google.cloud.bigquery.storage.v1beta1.ISplitReadStreamRequest = {}; | ||
request.originalStream = {}; | ||
request.originalStream.name = ''; | ||
// Mock response | ||
const expectedResponse = {}; | ||
// Mock gRPC layer | ||
client._innerApiCalls.splitReadStream = mockSimpleGrpcMethod( | ||
request, | ||
|
@@ -372,8 +367,6 @@ describe('v1beta1.BigQueryStorageClient', () => { | |
request.readPosition = {}; | ||
request.readPosition.stream = {}; | ||
request.readPosition.stream.name = ''; | ||
// Mock response | ||
const expectedResponse = {}; | ||
// Mock gRPC layer | ||
client._innerApiCalls.readRows = mockServerStreamingGrpcMethod(request, null, error); | ||
const stream = client.readRows(request); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
// ** All changes to this file may be overwritten. ** | ||
|
||
import * as gax from 'google-gax'; | ||
import {APICallback, Callback, CallOptions, Descriptors, ClientOptions, LROperation, PaginationCallback, PaginationResponse} from 'google-gax'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmmm. Maybe it's better to start actually using these types if possible? :) (in the client template, line 551 and nearby) |
||
import {APICallback, Callback, CallOptions, Descriptors, ClientOptions, LROperation} from 'google-gax'; | ||
import * as path from 'path'; | ||
|
||
import { Transform } from 'stream'; | ||
|
@@ -185,6 +185,7 @@ export class EchoClient { | |
// an Operation object that allows for tracking of the operation, | ||
// rather than holding a request open. | ||
const protoFilesRoot = opts.fallback? | ||
/* eslint-disable @typescript-eslint/no-var-requires */ | ||
this._gaxModule.protobuf.Root.fromJSON(require("../../protos/protos.json")) : | ||
this._gaxModule.protobuf.loadSync(nodejsProtoPath); | ||
|
||
|
@@ -237,6 +238,7 @@ export class EchoClient { | |
this.echoStub = this._gaxGrpc.createStub( | ||
this._opts.fallback ? | ||
(this._protos as protobuf.Root).lookupService('google.showcase.v1beta1.Echo') : | ||
/* eslint-disable @typescript-eslint/no-explicit-any */ | ||
// tslint:disable-next-line no-any | ||
(this._protos as any).google.showcase.v1beta1.Echo, | ||
this._opts) as Promise<{[method: string]: Function}>; | ||
|
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.
Do we need both
eslint-disable
andtsline:disable
?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.
Right! :) we just need eslint-disable, thank you! also removed tslint in base PR, thanks!
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.
it does not work without the latest version of gts. This PR was intended to make some pure template change before updating gts, but seems this is more and more overlapped with the gts PR, I may close this later on and resolve all related problems in the base PR: #343