Skip to content

test(admin-controller): add unit tests covering all controller endpoints#2544

Open
cristianscheid wants to merge 1 commit into
masterfrom
test/noid/admin-controller
Open

test(admin-controller): add unit tests covering all controller endpoints#2544
cristianscheid wants to merge 1 commit into
masterfrom
test/noid/admin-controller

Conversation

@cristianscheid

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

Refactor testCirclesList() and add coverage for all remaining endpoints in lib/Controller/AdminController.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
@cristianscheid cristianscheid force-pushed the test/noid/admin-controller branch from 1f8ae82 to e3a72bc Compare June 19, 2026 14:38

@come-nc come-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it’s the right kind of tests.
You could use assertSame directly on the array instead of each key/value I think.

/** @var CircleService|MockObject */
private $circleService;
foreach ([self::TEST_USER_1, self::TEST_USER_2] as $userId) {
$user = $c->get(IUserManager::class)->get($userId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should get the user manager only once and store it in a var

}

public function testDestroy(): void {
$c = $this->container;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
$c = $this->container;
$circleService = $this->container->get(CircleService::class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same in the other tests

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