Skip to content

Integration Tests: Avoid asserting on errors for permission tests#20643

Merged
AndyButland merged 2 commits intomainfrom
v17/hotfix/fix-integration-tests
Nov 11, 2025
Merged

Integration Tests: Avoid asserting on errors for permission tests#20643
AndyButland merged 2 commits intomainfrom
v17/hotfix/fix-integration-tests

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Resolves: #20639

Description

This PR fixes where we can - and comments where we can't - controller permission integration tests that were asserting on an error response.

There is an actual error revealed by one of these and noted in the linked issue, which is fixed and specific integration tests around this component - PropertyTypeUsageService - have been added.

Testing

Verify integration tests pass.

Copilot AI review requested due to automatic review settings October 24, 2025 12:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses flaky integration tests that were incorrectly asserting on error responses instead of success responses. The key change involves fixing the PropertyTypeUsageService implementation that was causing actual errors, and updating test assertions to expect HttpStatusCode.OK instead of InternalServerError where appropriate.

Key changes:

  • Fixed PropertyTypeUsageRepository to use WhereIn instead of Contains for database queries, resolving the underlying bug
  • Added new integration tests for PropertyTypeUsageService to verify the fix
  • Updated multiple controller permission tests to assert on HttpStatusCode.OK instead of error responses

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
PropertyTypeUsageServiceTests.cs New integration tests verifying the service correctly handles property value checks and content type validation
UmbracoIntegrationTestWithContent.cs Extracted content type key constant for reuse in tests
PropertyTypeUsageRepository.cs Fixed database query to use WhereIn instead of Contains, added XML documentation, and improved code formatting
PropertyTypeUsageService.cs Removed unused _contentTypeService field and marked parameter for future removal, added XML documentation
IPropertyTypeUsageService.cs Added XML documentation for interface
IPropertyTypeUsageRepository.cs Added XML documentation for repository interface methods
InviteUserControllerTests.cs Added clarifying comment explaining why InternalServerError is expected
IsUsedPropertyTypeControllerTests.cs Changed assertion from InternalServerError to OK after underlying bug fix
ValidateLogFileSizeLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
LogLevelCountLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
AllMessageTemplateLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
AllLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
ExecuteActionHealthCheckControllerTests.cs Changed assertion from InternalServerError to OK and added required Alias property to request

@AndyButland AndyButland force-pushed the v17/hotfix/fix-integration-tests branch from 0c838b9 to c3cead9 Compare October 24, 2025 12:23
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

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

Looks good to me, it has been a while since this was opened, so probably better switch the main branch to main 😁

@AndyButland AndyButland changed the base branch from release/17.0 to main November 11, 2025 05:25
@AndyButland AndyButland enabled auto-merge (squash) November 11, 2025 05:26
@AndyButland AndyButland merged commit cfa32b2 into main Nov 11, 2025
21 of 22 checks passed
@AndyButland AndyButland deleted the v17/hotfix/fix-integration-tests branch November 11, 2025 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration Test Issues

2 participants