-
Notifications
You must be signed in to change notification settings - Fork 106
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
fix: Check property existence for exclude from indexes with wildcard #1114
Merged
danieljbruce
merged 40 commits into
googleapis:main
from
danieljbruce:check-property-existence-for-excludeFromIndexes-with-wildcard
Sep 29, 2023
Merged
Changes from 34 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
e02139b
Add a test for testing without mocks
danieljbruce f139770
Do a refactor on the test
danieljbruce c8dd740
improve the test so that the error bubbles up
danieljbruce bdbe4c7
isolate the latter test right now
danieljbruce 24975b1
test fixes
danieljbruce d129b17
Add two more tests for excludeFromIndexes on array
danieljbruce 860afcb
Move the tests over to the index file
danieljbruce 099e8ef
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce 73cb4f1
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce 1b19906
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce be6b20e
PR update
danieljbruce 87a3f80
Merge branch 'check-property-existence-for-excludeFromIndexes-with-wi…
danieljbruce f0900e6
Create a function to refactor the tests
danieljbruce 3e1fa90
All tests for this feature now use same script
danieljbruce e43a1c3
Add parameterized testing
danieljbruce b21bd99
Eliminate function and inline code
danieljbruce 234f501
Add JSON stringify to test title
danieljbruce 17a5f2d
Organize code so that similar variables are used
danieljbruce aedf0c9
inline getExpectedConfig function
danieljbruce af1130f
Separate tests into blocks and inline code
danieljbruce b127359
Add types to parameters passed into async
danieljbruce 66d0a37
Eliminate unnecessary variable assignment
danieljbruce 94d7a24
Ran linter and simplified source changes
danieljbruce d9da7da
Add blank line back for easier diff reading
danieljbruce 0cd7cbc
Try again, eliminate the blank line
danieljbruce 377770b
Additional adjustment to entity first path part
danieljbruce df3f376
Define isFirstPathPartUndefined
danieljbruce 4f54d94
Rename the variable to align with what it does
danieljbruce a101f27
run linter
danieljbruce 2253740
Revert "run linter"
danieljbruce 8db651a
Revert "Rename the variable to align with what it does"
danieljbruce b798ec3
Revert "Define isFirstPathPartUndefined"
danieljbruce 3133879
Refactor check out for seeing if defined
danieljbruce b148c95
Move comment to more appropriate place
danieljbruce e0dfef3
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce d1ff7d6
Replace comments with description
danieljbruce 620ee00
Eliminate prefix. Only use description
danieljbruce e0d770f
fix typo
danieljbruce e1aecb6
lowercase convention
danieljbruce d75a732
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
What if we added a
description
property to eachonSaveTest
object with a human-readable, clear description of each scenario (it's probably fine to just reuse the wording for the comments on top of each object definition) and use it here instead of just dropping the blob ofJSON.stringify()
which I'm afraid will get really unreadable and harder to maintain with time.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.
Sounds good. Then we can use the description as the test name.
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.
Done