Conversation
Up to standards ✅🟢 Issues
|
…ttribute syntax Agent-Logs-Url: https://github.com/thomhurst/TUnit/sessions/31a2fe52-f921-4815-a7b3-4f0eaae13b49 Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This is a documentation-only PR that clarifies the purpose of MethodDataSourceAttribute.Factory in the XML doc comments.
Summary of changes
The PR replaces a one-line summary (Gets or sets an AOT-safe factory function for providing test data programmatically. When set, this factory is used instead of reflection-based member lookup.) with an expanded <remarks> block that explains:
- The property is populated automatically by the TUnit source generator — it cannot be set in attribute syntax because
Func<T>is not a C# attribute-legal value. - The source generator emits a delegate that calls the named method directly, enabling AOT/trimming compatibility.
Assessment
The change is correct and the new documentation is accurate. Looking at the implementation in GetDataRowsAsync, Factory is indeed checked first (if (Factory != null)) and the reflection-based path only runs when it is null, which is consistent with the description.
A few minor observations:
Accuracy — the summary line after the change reads "Gets or sets an AOT-safe factory function that provides test data without reflection." The "Gets or sets" phrasing is technically accurate (it is a settable property) but slightly misleading alongside the remark that manual assignment is not possible in attribute syntax. It is still settable via post-construction object initializer syntax, so the phrasing is not wrong. If the intent is to further discourage external setting, adding [EditorBrowsable(EditorBrowsableState.Never)] to the property would hide it from IntelliSense completion without changing runtime behavior — but that is a separate concern outside the scope of this PR.
Missing <example> block — the sibling properties (Arguments, SkipIfEmpty) and the class-level doc include <example> elements. The Factory property does not have one here, but given that the source generator manages this property exclusively, there is no useful user-facing example to add, so omitting it is fine.
Formatting — the use of <strong> in an XML doc <remarks> block renders correctly in generated API docs (DocFX, Sandcastle) and in VS tooltip tooltips (via the bold rendering in the IDE); no concern there.
Overall, this is a clear, accurate, and well-scoped documentation improvement. No issues found.
MethodDataSourceAttribute.Factoryconfused users into thinking it could be set in attribute syntax — it can't, since C# attributes only accept literal values andFunc<T>is not one. The property exists solely for the TUnit source generator to populate at compile time with a generated delegate that calls the named method directly (enabling AOT/trimming compatibility without reflection).Changes
MethodDataSourceAttribute.FactoryXML doc — replaced the terse description with an expanded<remarks>block that explains: