Skip to content

Conversation

@printminion-co
Copy link

Added comprehensive unit tests for the UserDeletedListener to ensure proper handling of user deletion events, including scenarios with no accounts, single and multiple accounts, client exceptions, and partial failures. This enhances test coverage and reliability of the mail account deletion process.

In order for us to extend the logic in the future

Added comprehensive unit tests for the UserDeletedListener to ensure proper handling of user deletion events, including scenarios with no accounts, single and multiple accounts, client exceptions, and partial failures. This enhances test coverage and reliability of the mail account deletion process.

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Thank you 🙏

Some smaller nitpicks, but overall a good initiative to add a test for it.

$this->assertInstanceOf(\OCP\EventDispatcher\IEventListener::class, $this->listener);
}

public function testHandleMethodHasOverrideAttribute(): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would you have a test for that?

);
}

private function createUserMock(string $userId = 'test-user'): IUser&MockObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private function createUserMock(string $userId = 'test-user'): IUser&MockObject {
private function createUserMock(string $userId): IUser&MockObject {

Not using a default makes the test easier to read because the value is given when used.

}

private function createAccountMock(int $id): Account&MockObject {
$account = $this->createMock(Account::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Account is mostly an DTO. Nicer would be, because we are not working with a test double, to instantiate a new object and only mock the MailAccount object.

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.

2 participants