-
Notifications
You must be signed in to change notification settings - Fork 119
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
Added count_documents() implementation to builtin_timeseries #935
Merged
+174
−0
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3c92c4b
Updated builtin_timeseries.py to add count_documents() implementation
776ba50
Fixed typo in variable name
c4ea9a8
Wrote a basic test case for count_data() in builtin_timeseries().py
e773c34
Changed count entries function_name and signature and modified test c…
bb88af4
Moved assert statements
821478c
Improvised Testcases for edge cases + Refactored logic
177c45b
Fixed Aggregate class test case
0903cf7
Updated code comments for TestTimeSeries.py
825d4ce
Simplified count computation and updated returned values
9d4063f
Removed test case for aggregate subclass with empty keys
ac619ba
- Fixed unaccounted entry related to tearDown() + Re-added aggregate …
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,13 @@ def setUp(self): | |
self.testUserId = uuid.uuid3(uuid.NAMESPACE_URL, "mailto:[email protected]") | ||
edb.get_analysis_timeseries_db().delete_many({'user_id': self.testUserId}) | ||
|
||
def tearDown(self): | ||
self.clearRelatedDb() | ||
|
||
def clearRelatedDb(self): | ||
edb.get_timeseries_db().delete_many({"user_id": self.testUserId}) | ||
edb.get_analysis_timeseries_db().delete_many({"user_id": self.testUserId}) | ||
|
||
def testGetLastPlace(self): | ||
old_place = ecwrp.Rawplace() | ||
old_place.enter_ts = 5 | ||
|
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.
This will mean that this test can never fail, so it is essentially the same as the test being commented out like it was before. It is true that you print out an error message if you catch the assertion, but who is checking for log statements in the automated tests, particularly while running in a Ci/CD environment?
The only thing that people pay attention to is the red or green badge 😄
If you have actually fixed the test, you should have confidence in it and fail the test if the assertion fails.
You can do this by removing the try/catch or by leaving this in for debugging, but rethrowing the error after printing out the CTE
If you do choose to keep the try/catch + rethrow, you should add a TODO: to remove it in Sept 2024, so that we don't have any more vestigial code sitting around.