Skip to content

Conversation

@WhitWaldo
Copy link
Contributor

@WhitWaldo WhitWaldo commented Feb 26, 2025

Description

While investigating #1464 , I discovered that there already existed several unit tests with one on point. While I'm unable to recreate the issue detailed, I did notice that there were a few improvements to make to the unit tests in this class, including:

  • The GetReminderAsync_ReturnsResultWhenAvailable test double checked the Period property instead of checking both Period and DueTime.
  • Several actor callbacks re-used the same variable names as outside the delegate.
  • Used const string where possible instead of var.
  • Removed unused usings
  • Removed unused variables in tests
  • Added Dapr copyright header

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1464

Checklist

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

  • Code compiles correctly
  • Created/updated tests
  • [N/A] Extended the documentation

…runs a duplicate check instead by accident)

Signed-off-by: Whit Waldo <[email protected]>
Signed-off-by: Whit Waldo <[email protected]>
Signed-off-by: Whit Waldo <[email protected]>
@WhitWaldo WhitWaldo added this to the v1.15 milestone Feb 26, 2025
@WhitWaldo WhitWaldo self-assigned this Feb 26, 2025
@WhitWaldo WhitWaldo requested review from a team as code owners February 26, 2025 11:13
@WhitWaldo WhitWaldo merged commit cb065f2 into dapr:master Feb 26, 2025
12 checks passed
var defaultActorTimerManager = new DefaultActorTimerManager(interactor.Object);
var actorReminder = new ActorReminder(actorType, new ActorId(actorId), "remindername", new byte[] { }, TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(1));
var actualData = string.Empty;
var actorId = "123";
Copy link
Contributor

@siri-varma siri-varma Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be a nitpick, is the indentation off here ? I see variables placed directly under the bracket (line 29)

nvm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deserialise error an Actor Reminder when using GetReminderAsync()

2 participants