Skip to content

Conversation

@TonyTroeff
Copy link
Member

Pull Request Description

This pull request introduces support for allowing multiple [CleanUtility(...)] annotations on the same class, enhancing the functionality and flexibility of the CleanUtility attribute.

Motivation and Context

Previously, it was only possible to use a single [CleanUtility(...)] annotation per class, which could be limiting for more complex scenarios requiring multiple attributes. This change provides more versatility.

Checklist

  • I have tested these changes thoroughly.
  • I have added/updated relevant documentation.
  • My code follows the project's coding guidelines.
  • I have performed a self-review of my changes.
  • My changes are backwards compatible.

@TonyTroeff TonyTroeff self-assigned this Sep 5, 2025
@TonyTroeff TonyTroeff added feature Something that should be implemented task A simple unit of work used to move the features forward labels Sep 5, 2025
@TonyTroeff TonyTroeff linked an issue Sep 5, 2025 that may be closed by this pull request
@TonyTroeff TonyTroeff added this to the 1.2.0 milestone Sep 5, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 5, 2025

Copy link

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 enables multiple [CleanUtility(...)] annotations on the same class by changing the AllowMultiple parameter from false to true in the CleanUtilityAttribute. The existing code in CleanTestFramework.cs already supports iteration over multiple attributes, so the change enables a previously disabled capability.

Changes:

  • Modified CleanUtilityAttribute to allow multiple instances per class (AllowMultiple = true)
  • Renamed local variable from initializationUtilityAttributes to cleanUtilityAttributes for better consistency with the attribute name

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
TryAtSoftware.CleanTests.Core/Attributes/CleanUtilityAttribute.cs Updated AttributeUsage to allow multiple annotations per class
TryAtSoftware.CleanTests.Core/XUnit/CleanTestFramework.cs Renamed variable for consistency (functionality already supported multiple attributes)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

using TryAtSoftware.Extensions.Collections;

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

While allowing multiple CleanUtility attributes on the same class is technically supported by the registration code, there's a critical constraint that needs to be documented: each attribute on the same class must have a unique combination of Category and Name. The utility ID is computed as "c:{Category}|n:{Name}" (see ICleanUtilityDescriptor interface), and duplicate IDs will cause a runtime exception during registration (see CleanTestAssemblyData constructor line 33: "Two clean utilities with the same identifier cannot co-exist."). Consider adding this constraint to the attribute's documentation or implementing validation to provide a clearer error message when duplicate combinations are detected.

Copilot uses AI. Check for mistakes.

var initializationUtilityAttributes = type.GetCustomAttributes(typeof(CleanUtilityAttribute)).ToArray();
if (initializationUtilityAttributes.Length == 0) continue;
var cleanUtilityAttributes = type.GetCustomAttributes(typeof(CleanUtilityAttribute)).ToArray();
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The variable has been renamed from 'initializationUtilityAttributes' to 'cleanUtilityAttributes' for consistency with the attribute name. However, this is only a partial renaming - the variable 'initializationUtility' at line 86 still uses the old naming convention. Consider renaming it to 'cleanUtility' or 'cleanUtilityDescriptor' for consistency throughout the method.

Copilot uses AI. Check for mistakes.
using TryAtSoftware.Extensions.Collections;

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

This feature enhancement lacks test coverage. While the existing code can handle multiple attributes through iteration, there are no tests validating this new capability. Consider adding tests that cover: (1) successfully registering a class with multiple CleanUtility attributes with different category/name combinations, (2) verifying that duplicate category/name combinations on the same class are properly rejected, and (3) ensuring that utilities registered through multiple attributes are correctly available for test discovery and execution.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Something that should be implemented task A simple unit of work used to move the features forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support to define multiple [CleanUtility] attributes

2 participants