Fixed CI/CD Tests for Edge-Endpoint #70
Conversation
…en IQ is created on the edge
| @@ -21,7 +21,7 @@ kubernetes = "^27.2.0" | |||
| jinja2 = "^3.1.2" | |||
| SQLAlchemy = "2.0.22" | |||
| APScheduler = "3.10.4" | |||
| groundlight = "^0.13.1" | |||
| groundlight = "^0.17.0" | |||
There was a problem hiding this comment.
This makes me happy
test/api/test_motdet.py
Outdated
| @@ -240,28 +240,40 @@ def test_motion_detection_not_sufficient_if_doesnt_meet_conf_threshold(gl: Groun | |||
| detector_id = DETECTORS["dog_detector"]["detector_id"] | |||
| detector = gl.get_detector(id=detector_id) | |||
|
|
|||
| # Set detector confidence threshold to 0.90 | |||
| gl.update_detector_confidence_threshold(detector.id, 0.90) | |||
There was a problem hiding this comment.
This is just to restore the detector to a known state, because we might change it in a previous test run?
There was a problem hiding this comment.
Yes, the purpose of this test is to try to see if the motion detector, even when there is no motion detected, will escalated to the cloud if the detector confidence threshold changed to a higher value than the first IQ's confidence.
test/api/test_motdet.py
Outdated
| if new_response.result is None or new_response.result.confidence is None or new_response.result.confidence > 0.95: | ||
| # Revert the confidence threshold to 0.90 | ||
| gl.update_detector_confidence_threshold(detector.id, 0.90) | ||
| pytest.skip("This test requires that the cached image query response has a confidence < 0.95") |
There was a problem hiding this comment.
It looks like this test skipped when you ran it, I'm not sure if we'll get to the following assertion which is the meat of what we're testing here. Is there a way to make this more reliable? Is it just a matter of raising the confidence even higher?
There was a problem hiding this comment.
Yea @tyler-romero and I found out that the test originally was made based on a misunderstanding of the SDK that the confidence_threshold will be forwarded to the API.
Since it is polling the client, it will just timeout if the original IQ is already high enough. The test right now using the dog detector can already achieved a confidence of 0.98.
If we want to make this test working for most scenarios, I can try to raise it to 0.99 with a wait time long enough to trigger a cloud labeler to respond.
|
After discussion with @tyler-romero, the test that failed is created from a SDK implementation misunderstanding of the confidence threshold will forward to the API (confidence threshold is a client only polling). Thus, it is safe for this test case to be removed. |
app/core/utils.py
Outdated
| confidence=confidence, | ||
| label=label, | ||
| ), | ||
| patience_time=30.0, |
There was a problem hiding this comment.
Was there a specific reason to add this? It should be taken as an argument to create_iqe and the true requested patience_time should be set here from post_image_query
There was a problem hiding this comment.
Yea I added when I was doing some testing and forget to add to post_image_query. Will add it back.
This PR contains the following changes to fix CI/CD tests failing:
motion-detectionnot passingAs a side note, for future improvements it might be better if we can either make sure the tests also passed for edge-endpoint if we update the SDK, or use a matching sdk tests for the sdk version the edge-endpoint is running if we are not updating edge's sdk version that often.