test: Break the ReadRows service down into classes instead of a single long function #1461
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only ignore keyFrom and keyTo when they are undefined and not just when they are Falsy.
The old service has been generalized enough to mock correct server behaviour.
they are undefined anyway.
…/github.com/googleapis/nodejs-bigtable into 3527322442-refactor-the-test-into-classes-2
…dejs-bigtable into 3527322442-refactor-the-test-into-classes-2
…/github.com/googleapis/nodejs-bigtable into 3527322442-refactor-the-test-into-classes-4 # Conflicts: # test/utils/readRowsImpl.ts
Add a factory method instead
product-auto-label
bot
added
size: l
danieljbruce
changed the base branch from
3527322442-to-main-2
to
3527322442-refactor-the-test-into-classes-3
danieljbruce
changed the title
product-auto-label
bot
added
size: m
danieljbruce
changed the base branch from
3527322442-refactor-the-test-into-classes-3
to
3527322442-to-main-2
…nodejs-bigtable into 3527322442-refactor-the-test-into-classes-4 # Conflicts: # test/readrows.ts # test/utils/readRowsImpl.ts
leahecole
approved these changes
Aug 16, 2024
danieljbruce
added a commit
that referenced
this pull request
Sep 23, 2024
* test: begin to refactor the mock service (#1447) * Use prettyPrintRequest from readRowsImpl1 * Define interfaces for creating a Bigtable service * Remove keyFrom and keyTo from each test * Define the service inline for standard keys errors * Add a comment about the refactor steps I am doing * Add a header to the service parameters file * Use the ChunkGeneratorParameters interface * Simplify the diff by creating local variables * Remove comment * Eliminate chunks per response constant * Change imports * Replace with as ServerImplementationInterface * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Add a second test for an error at a random pos * Add some comments for documentation * Chunk generation - add the parameter * Added comments to all the interfaces * Fix a regression bug from the merge --------- Co-authored-by: Leah E. Cole <[email protected]> * test: Eliminate the second ReadRows Service and use a generalized version of the first ReadRows (#1457) * Use prettyPrintRequest from readRowsImpl1 * Define interfaces for creating a Bigtable service * Remove keyFrom and keyTo from each test * Define the service inline for standard keys errors * Add a comment about the refactor steps I am doing * Add a header to the service parameters file * Use the ChunkGeneratorParameters interface * Simplify the diff by creating local variables * Remove comment * Eliminate chunks per response constant * Change imports * Set up the second ReadRows service to use params * Remove duplicate copy of generate chunks from serv * Remove second copy of isKeyInRowSet * Eliminate duplicate copies of the debug log * Fix a bug for the to and from service parameters Only ignore keyFrom and keyTo when they are undefined and not just when they are Falsy. * Add cancel back into the mock service for testing * Add variables to match service 1 * Add one more check to match the other service * Remove usages of ReadRows2Impl * Remove the new service The old service has been generalized enough to mock correct server behaviour. * Moved the position of the comment for 150 rows * Eliminate setting keyTo and keyFrom they are undefined anyway. * Add a second test for an error at a random pos * Add some comments for documentation * Chunk generation - add the parameter * Added comments to all the interfaces * Delete file * Change splitted to split * Remove export * Don’t rename the interfaces There is no point * Increase Windows timeout * Provide functions to eliminate if statements * Eliminate the if statements Make a direct call to generateChunksFromRequest * Revert "Eliminate the if statements" This reverts commit 0996e89. * Revert "Provide functions to eliminate if statements" This reverts commit 4a4761f. * Change any’s to string | undefined * Eliminate duplicate code for setting timeouts * remove only * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> * Update test/readrows.ts Co-authored-by: Leah E. Cole <[email protected]> --------- Co-authored-by: Leah E. Cole <[email protected]> * test: Eliminate repeated if statements in the ReadRows mock service reducing the size of it significantly (#1460) * Use prettyPrintRequest from readRowsImpl1 * Define interfaces for creating a Bigtable service * Remove keyFrom and keyTo from each test * Define the service inline for standard keys errors * Add a comment about the refactor steps I am doing * Add a header to the service parameters file * Use the ChunkGeneratorParameters interface * Simplify the diff by creating local variables * Remove comment * Eliminate chunks per response constant * Change imports * Set up the second ReadRows service to use params * Remove duplicate copy of generate chunks from serv * Remove second copy of isKeyInRowSet * Eliminate duplicate copies of the debug log * Fix a bug for the to and from service parameters Only ignore keyFrom and keyTo when they are undefined and not just when they are Falsy. * Add cancel back into the mock service for testing * Add variables to match service 1 * Add one more check to match the other service * Remove usages of ReadRows2Impl * Remove the new service The old service has been generalized enough to mock correct server behaviour. * Moved the position of the comment for 150 rows * Eliminate setting keyTo and keyFrom they are undefined anyway. * Add a second test for an error at a random pos * Add some comments for documentation * Chunk generation - add the parameter * Added comments to all the interfaces * Group the property getter into a function * Group key selection into functions * Fix typo: default key * Documentation for isInRowSet * Eliminate variable - move function inline * Omit optional selector on stream * Create ReadRowsWritableStream interface * Use new interface, remove ServerWritableStream * Don’t pass the whole stream into helpers * Add a function for generating the chunks * The debug log should be a pass through parameter * Solve compiler errors resulting immediately from merge * Add debugLog parameter to various functions * Add return type * Remove exports - functions are only used in this module * Revise merge correction * Remove TODO for the stream type * Update the getKeyValue function * Eliminate place where debug log is needed * Run linter * test: Break the ReadRows service down into classes instead of a single long function (#1461) * Use prettyPrintRequest from readRowsImpl1 * Define interfaces for creating a Bigtable service * Remove keyFrom and keyTo from each test * Define the service inline for standard keys errors * Add a comment about the refactor steps I am doing * Add a header to the service parameters file * Use the ChunkGeneratorParameters interface * Simplify the diff by creating local variables * Remove comment * Eliminate chunks per response constant * Change imports * Set up the second ReadRows service to use params * Remove duplicate copy of generate chunks from serv * Remove second copy of isKeyInRowSet * Eliminate duplicate copies of the debug log * Fix a bug for the to and from service parameters Only ignore keyFrom and keyTo when they are undefined and not just when they are Falsy. * Add cancel back into the mock service for testing * Add variables to match service 1 * Add one more check to match the other service * Remove usages of ReadRows2Impl * Remove the new service The old service has been generalized enough to mock correct server behaviour. * Moved the position of the comment for 150 rows * Eliminate setting keyTo and keyFrom they are undefined anyway. * Add a second test for an error at a random pos * Add some comments for documentation * Chunk generation - add the parameter * Added comments to all the interfaces * Group the property getter into a function * Group key selection into functions * Fix typo: default key * Documentation for isInRowSet * Eliminate variable - move function inline * Omit optional selector on stream * Create ReadRowsWritableStream interface * Use new interface, remove ServerWritableStream * Don’t pass the whole stream into helpers * Add a function for generating the chunks * The debug log should be a pass through parameter * Add some TODOs about how to address this next * Pull send response into a class * Create a dedicated class for defining the service * Change name to readRowsRequestHandler * Pull the function that generates the chunks into Separate module * Generate documentation * Add more documentation to the Service class * Add debugLog as a method parameter * Solve compiler errors resulting immediately from merge * Add debugLog parameter to various functions * Add return type * Remove exports - functions are only used in this module * Revise merge correction * Remove TODO for the stream type * Update the getKeyValue function * Eliminate place where debug log is needed * Run linter * Eliminate the function that creates a service Add a factory method instead * Add documentation for the constructor * Remove the TODO * Set the timeout for the test * Increase the timeout * Eliminate ternary statement * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Add a couple of comments * re-write the ternary expression --------- Co-authored-by: Leah E. Cole <[email protected]> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This PR replaces the long and complex
readRowsImplfunction with a group of classes containing smaller functions for handling a ReadRows request.Background:
This is the final PR for making the ReadRows service more modular. So far three PRs have been written in order to make the ReadRows test service more modular:
Changes:
ReadRowsImplclass for defining the Bigtable ReadRows service. Make the constructor of the class private to control the context in which the class is created and add a staticcreateServicemethod to the class which will use the constructor to create the service.readRowsImplfunction with a call tocreateServicenow thatreadRowsImplhas been deleted.ReadRowsRequestHandlerand another isReadRowsImpl.ReadRowsImplwill store data that changes between requests. In particular,errorAfterChunkNois data that changes between requests.ReadRowsImpluses aReadRowsRequestHandlerobject when handling an incoming request.ReadRowsRequestHandlerwill maintain information about whether the stream is waiting/cancelled and will have one function used for sending responses. This function sendResponse will do the same thing that the sendResponse function did before.ReadRowsImpland be broken up into ahandleRequestfunction which calls a sendAllChunks function.