Skip to content

test: replace checkbox assertions with DoesNotPerformAssertions (misc)#60747

Draft
miaulalala wants to merge 1 commit into
masterfrom
fix/noid/checkbox-tests-misc
Draft

test: replace checkbox assertions with DoesNotPerformAssertions (misc)#60747
miaulalala wants to merge 1 commit into
masterfrom
fix/noid/checkbox-tests-misc

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

Summary

Replaces addToAssertionCount(1) and assertFalse(false) placeholder assertions with #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions] across 15 unrelated test files.

  • 13 tests use addToAssertionCount(1) solely to suppress the PHPUnit "risky test" warning — they verify only that no exception is thrown
  • 2 tests in EncryptionTest use assertFalse(false) as a placeholder, but both already have real expects($this->once()) mock expectations that serve as the meaningful assertion
  • The "Doesn't assert anything" docblock comments in CrashReport/RegistryTest and Subscription/RegistryTest are removed since #[DoesNotPerformAssertions] communicates the same intent

Part of a broader effort to eliminate checkbox tests across the test suite (see also #60742).

Test plan

  • Run the affected test classes locally to confirm no regressions
  • CI green

🤖 Generated with Claude Code

@miaulalala miaulalala self-assigned this May 26, 2026
@miaulalala miaulalala added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 tests Related to tests labels May 26, 2026
@miaulalala miaulalala added this to the Nextcloud 35 milestone May 26, 2026
miaulalala added a commit that referenced this pull request May 26, 2026
… (middleware)

Replace `addToAssertionCount(1)` placeholders in AppFramework middleware
tests that only verify no exception is thrown.

For `SameSiteCookieMiddlewareTest`, two tests already have
`expects(self::once())` mock expectations as their real assertion — those
just have the redundant placeholder removed. The third test has no
expectations and gets `#[DoesNotPerformAssertions]`.

For `SecurityMiddlewareTest`, `testIsSubAdminCheck` and
`testIsSubAdminAndAdminCheck` get `#[DoesNotPerformAssertions]`.
`testRestrictedAppLoggedInPublicPage` and `testRestrictedAppNotLoggedInPublicPage`
have `->with()` argument constraints that count as assertions in PHPUnit 11,
so the redundant placeholder is simply removed.

Part of a broader effort to eliminate checkbox tests (see also #60742, #60747).

Signed-off-by: Anna Larch <anna@nextcloud.com>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@miaulalala miaulalala force-pushed the fix/noid/checkbox-tests-misc branch from 1bec2b5 to 87230b3 Compare May 26, 2026 21:18
@miaulalala miaulalala changed the title fix(tests): replace checkbox assertions with DoesNotPerformAssertions (misc) test: replace checkbox assertions with DoesNotPerformAssertions (misc) May 26, 2026
@miaulalala
Copy link
Copy Markdown
Contributor Author

/backport to stable34

@miaulalala miaulalala force-pushed the fix/noid/checkbox-tests-misc branch from 87230b3 to 2c8f7a9 Compare May 28, 2026 08:20
Signed-off-by: Anna Larch <anna@nextcloud.com>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@miaulalala miaulalala force-pushed the fix/noid/checkbox-tests-misc branch from 2c8f7a9 to 2cc7f76 Compare May 28, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted backport-request technical debt 🧱 🤔🚀 tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant