dapr / go-sdk

Dapr SDK for go
Apache License 2.0
441 stars 170 forks source link

Add tests for stateManagerCtx #558

Open KentHsu opened 2 months ago

KentHsu commented 2 months ago

Description

This PR add tests for stateManagerCtx mockgen a mock_client file so stateManagerCtx can run tests with this mock client Added tests for methods in stateManagerCtx and fixed a bug in setWithTTL method

Issue reference

Please reference the issue this PR will close: #446

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 4.77137% with 479 lines in your changes are missing coverage. Please review.

Project coverage is 55.92%. Comparing base (27248ba) to head (be6e8ce). Report is 10 commits behind head on main.

:exclamation: Current head be6e8ce differs from pull request most recent head 40c60b0. Consider uploading reports for the commit 40c60b0 to get more accurate results

Files Patch % Lines
actor/mock_client/mock_client.go 4.65% 471 Missing :warning:
actor/mock/mock_server.go 0.00% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #558 +/- ## ========================================== - Coverage 58.04% 55.92% -2.13% ========================================== Files 55 56 +1 Lines 3568 4070 +502 ========================================== + Hits 2071 2276 +205 - Misses 1375 1665 +290 - Partials 122 129 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KentHsu commented 2 months ago

Thank you for the feedback. I have updated this PR with

mikeee commented 2 months ago

Just had another skim-read, looking great so far. I haven't forgotten and will pick the review up tomorrow when I get a moment 👌

KentHsu commented 2 months ago

Sure. Please take your time 👍 BTW, I realize that I do need another PR to update .codecov.yaml... I'll submit another PR to update it when this one is ready.

mikeee commented 2 months ago

Is there any reason against using Uber's maintained fork to generate mock interfaces?

KentHsu commented 2 months ago

I tried both and I didn't see anything blocking us from using Uber's fork. If we want to switch to Uber's fork, I'll have to update go.mod and other test files using these mocks since Uber's fork doesn't work with those original mocks. Is that okay?

KentHsu commented 2 months ago

I went ahead to make some commits to use Uber's gomock fork and revert the change in .codecov.yaml. Please let me know if there's any concern. Thanks.

KentHsu commented 1 month ago

Hi @mikeee, would you like me to create another PR to exclude the mock files so that this PR won't be block by code coverage? Any suggestions on the code change in this PR?