chore(testproxy): fix close generated (v2) clients #1249
Conversation
Tests for closing the client are failing since a new client object is created on every operation that uses the generated (v2) layer. This changeset fixes it by moving the v2 client creation to `testproxy/services/create-client.js` alongside the veneer client creation. Every command using the v2 layer is updated to read the client from the client map instead of creating a new instance. Three previosly failing tests are now passing with this fix: - TestCheckAndMutateRow_Generic_CloseClient - TestMutateRow_Generic_CloseClient - TestReadModifyWriteRow_Generic_CloseClient
| const closeClient = ({clientMap}) => | ||
| normalizeCallback(async rawRequest => { | ||
| const request = rawRequest.request; | ||
| const {clientId} = request; | ||
| const bigtable = clientMap.get(clientId); | ||
|
|
||
| if (bigtable) { | ||
| bigtable.close(); | ||
| await bigtable[v2].close(); |
There was a problem hiding this comment.
I actually think this line might not be necessary.
There was a problem hiding this comment.
Good point but I just confirmed it's necessary. I tried running the tests without closing the v2 instance (removing this line) and tests fail again without it.
My understanding is that veneer and v2 are completely independent instances so it makes sense that it needs to be closed separately.
There was a problem hiding this comment.
Got it. If that is the case then it looks like the code contains a client bug because closing the veneer is supposed to close the clients on the v2 layer. Oh, but the client never gets initialized because we never make calls with the veneer layer and clients are initialized lazily. I suspect though that when we use the veneer layer for the mutateRow calls when mutateRow is exposed then we will not need this line anymore.
Thanks for checking.
Tests for closing the client are failing since a new client object is created on every operation that uses the generated (v2) layer. This changeset fixes it by moving the v2 client creation to
testproxy/services/create-client.jsalongside the veneer client creation and properly closing that v2 instance ontestproxy/services/close-client.js.Every command using the v2 layer is updated to read the client from the client map instead of creating a new instance.
Three previosly failing tests are now passing with this fix: