test: Eliminate repeated if statements in the ReadRows mock service reducing the size of it significantly#1460
Merged
danieljbruce merged 50 commits into3527322442-to-main-2from Aug 12, 2024
Conversation
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
danieljbruce
commented
Jul 29, 2024
| console.log(text); | ||
| } | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
DebugLog is just being moved from the service to here in the test
danieljbruce
commented
Jul 29, 2024
| if (DEBUG) { | ||
| console.log(text); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This has just been moved to the test and will be passed into the service.
danieljbruce
commented
Jul 29, 2024
| chunkSize: serviceParameters.chunkSize, | ||
| valueSize: serviceParameters.valueSize, | ||
| }); | ||
| const chunks = generateChunksFromRequest(stream.request, serviceParameters); |
Contributor
Author
There was a problem hiding this comment.
This single line of code is a substitute for all of the code deleted previous to it.
Contributor
Author
There was a problem hiding this comment.
The code for each if statement now lives in getKeyValue. generateChunksFromRequest calls generateChunks making 2 calls to getSelectedKey each making 2 calls to getKeyValue.
bhshkh
approved these changes
Aug 9, 2024
leahecole
approved these changes
Aug 12, 2024
danielbankhead
approved these changes
Aug 12, 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 <6719667+leahecole@users.noreply.github.com> * 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 <6719667+leahecole@users.noreply.github.com> * 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 <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * Update test/readrows.ts Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> --------- Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com> * 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 <6719667+leahecole@users.noreply.github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
4 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 reduces all of those custom
ifstatements down to one line of code in the ReadRows mock implementation cutting the ReadRows mock implementation down to about half the size so that in the next PR it is easy to make a modular version of the ReadRows mock implementation.Background:
As requested, the eventual goal is to address these TODOs. So far, two PRs have already been merged to take steps toward this:
Changes:
ReadRowsWritableStreamfor theServerWritableStream<x,x>type used everywhere for conciseness. Then replace all references toServerWritableStream<x,x>with theReadRowsWritableStreamtype.generateChunksFromRequest.generateChunksFromRequestthen calls a newgenerateChunksfunction with calculatedkeyToandkeyFromvalues.keyToandkeyFromare both calculated using a newgetSelectedKeymethod which uses a default key if it is provided and otherwise uses the key provided in the open/close properties of the request.Next Steps:
The
readRowsImplfunction will be broken down into classes as required by the TODO.