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

refactor: major generated test refactor, gts-2.0-compatible#365

Merged
alexander-fenster merged 1 commit intomasterfrom
next-line
Mar 27, 2020
Merged

refactor: major generated test refactor, gts-2.0-compatible#365
alexander-fenster merged 1 commit intomasterfrom
next-line

Conversation

@alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Mar 27, 2020

Fixes googleapis/google-cloud-node-core#659.

So... this started as an attempt to make the generated tests just a little bit less ugly, but... here we go, complete refactor, 98+% coverage, proper TypeScript import, async / await in tests, and pure sinon with no weird manual mocks.

Just one example. Before:

function mockSimpleGrpcMethod(expectedRequest: {}, response: {} | null, error: FakeError | null) {	
    return (actualRequest: {}, options: {}, callback: Callback) => {	
        assert.deepStrictEqual(actualRequest, expectedRequest);	
        if (error) {	
            callback(error);	
        } else if (response) {	
            callback(null, response);	
        } else {	
            callback(null);	
        }	
    };	
}

Now:

function stubSimpleCall<ResponseType>(response?: ResponseType, error?: Error) {
    return error ? sinon.stub().rejects(error) : sinon.stub().resolves([response]);
}

function stubSimpleCallWithCallback<ResponseType>(response?: ResponseType, error?: Error) {
    return error ? sinon.stub().callsArgWith(2, error) : sinon.stub().callsArgWith(2, null, response);
}

Benefits of having good test coverage: I fixed 3 or 4 typing issues in the client :)

Sinon rocks.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 27, 2020
@alexander-fenster alexander-fenster force-pushed the next-line branch 3 times, most recently from a938ac9 to eeb805f Compare March 27, 2020 11:05
Copy link
Contributor

@xiaozhenliu-gg5 xiaozhenliu-gg5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! this is awesome!

@alexander-fenster alexander-fenster merged commit 9b83da1 into master Mar 27, 2020
@alexander-fenster alexander-fenster deleted the next-line branch March 27, 2020 17:44
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.

lint: replace require with import in test

5 participants