test(services): add direct unit tests for cloudwatch_client#1032
test(services): add direct unit tests for cloudwatch_client#1032muddlebee merged 3 commits intoTracer-Cloud:mainfrom
Conversation
Greptile SummaryAdds 18 direct unit tests for Confidence Score: 4/5Safe to merge once the credentials tests are tightened; all previous review concerns have been addressed. All prior P1/P2 findings (module-level import, .kwargs access, missing ClientError error-message assertions) are resolved. One remaining P2 concern: the three credentials tests don't mock the client helper, so the assertion tests/services/test_cloudwatch_client.py — credentials test methods in all three classes Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Test calls service function] --> B{client fixture\nrequested?}
B -->|Yes| C[_get_cloudwatch_client returns MagicMock]
B -->|No - credentials tests| D[_get_cloudwatch_client called for real]
C --> E[require_aws_credentials checked]
D --> E
E -->|test overrides patch| F[Returns error dict]
E -->|autouse patch returns None| G[Proceeds to boto3 call]
F --> H[Assert success is False\nWARNING: no error-message assertion]
G --> I{Simulated boto3 call}
I -->|ClientError| J[Returns error dict\nAssert success + error message]
I -->|return_value| K[Returns success dict\nAssert success + data]
Reviews (2): Last reviewed commit: "Address Greptile review: hoist imports, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@GoDiao fix ci and review |
|
@GoDiao greptile reviews not yet fixed. Could you pls handle them. Pls check carefully |
…ent error assertions
|
@GoDiao thank you 🌟 |
|
@muddlebee Sorry for the back-and-forth! I'm new to open source contributions and still learning the ropes — didn't mean to waste your time with the lint issues and Greptile review items. All three Greptile suggestions should be addressed now (hoisted the ClientError import, switched to call_args.kwargs, and added consistent error-message assertions). Also fixed the ruff lint CI failure. Thanks for your patience! |
|
@GoDiao no issues, nicely done. Hope you are having good time contributing :) |
|
feel free to join our discord if you haven't already https://discord.com/invite/opensre |
2 participants
Fixes #885
Describe the changes you have done in this PR -
Added
tests/services/test_cloudwatch_client.pywith 18 direct unit tests for the three service functions inapp/services/cloudwatch_client.py:All tests mock
make_boto3_client()andrequire_aws_credentials()as specified in the issue. Tool-registration assertions are intentionally excluded.Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Each service function (
get_metric_statistics,filter_log_events,get_log_events) follows the same pattern: get a boto3 client → check credentials → call AWS API → return success/error. Tests mirror this pattern with three scenarios per function: happy path with correct parameter forwarding, error handling (ClientError / no client / no credentials), and optional parameter handling. I usedunittest.mock.patchon the internal_get_cloudwatch_clientand_get_cloudwatch_logs_clienthelpers to keep mocking minimal and focused.Checklist before requesting a review