Skip to content

feat: Add Asset handling#1847

Open
linkdotnet wants to merge 1 commit into
mainfrom
feat/#1846
Open

feat: Add Asset handling#1847
linkdotnet wants to merge 1 commit into
mainfrom
feat/#1846

Conversation

@linkdotnet
Copy link
Copy Markdown
Collaborator

This fixes #1846

The basic idea is to have a convenient AddAsset method that passes a collection (just like AddParameter) to the Renderer - ComponentBase then can "grab" those.

This allow emulation of "@assets"

Copy link
Copy Markdown

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for seeding .NET 9+ ComponentBase.Assets in bUnit via a new AddAsset API, enabling tests to emulate @Assets/MapStaticAssets behavior.

Changes:

  • Introduces BunitContext.AddAsset and renderer-side asset storage to expose a ResourceAssetCollection through ComponentBase.Assets.
  • Adds new test assets/components and a new test suite covering indexer behavior, iteration, and custom properties.
  • Documents the feature (“Seeding static assets”) and links it from the docs TOC and changelog.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/bunit.tests/Rendering/AssetsTest.razor New tests validating default/seeded Assets behavior via AddAsset.
tests/bunit.testassets/Assets/AssetsPropertiesComponent.razor Test component for iterating assets and rendering properties.
tests/bunit.testassets/Assets/AssetsIterationComponent.razor Test component for reading label property from assets and rendering labels.
tests/bunit.testassets/Assets/AssetsIndexerComponent.razor Test component validating Assets["key"] indexer mapping.
src/bunit/Rendering/BunitRenderer.cs Adds backing store + override for Assets and implements AddAsset.
src/bunit/BunitContext.cs Exposes AddAsset on BunitContext delegating to the renderer.
docs/site/docs/toc.md Adds link to new “Seeding static assets” documentation.
docs/site/docs/providing-input/seeding-assets.md New documentation explaining how to seed Assets in tests.
CHANGELOG.md Notes addition of AddAsset in “Added”.

Comment on lines +1 to +6
@code{
#if NET9_0_OR_GREATER
}
@using Bunit.TestAssets.Assets;
@inherits BunitContext
@code {
Comment on lines +84 to +86
@code{
#endif
}
Comment on lines +104 to +115
public void AddAsset(string url, string? label = null, params ResourceAssetProperty[] properties)
{
ArgumentException.ThrowIfNullOrEmpty(url);
ArgumentNullException.ThrowIfNull(properties);

var props = new List<ResourceAssetProperty>(properties.Length + 1);
if (label is not null)
{
props.Add(new ResourceAssetProperty("label", label));
}

props.AddRange(properties);
Comment on lines +16 to +19
=> SubresourceNames = Assets
.Select(asset => asset.Properties?.SingleOrDefault(property => property.Name == "label")?.Value)
.OfType<string>()
.ToList();
Comment thread src/bunit/BunitContext.cs
Comment on lines +192 to +195
/// <remarks>
/// Pass a <paramref name="label"/> to map a stable asset key to its (fingerprinted) <paramref name="url"/>,
/// i.e. <c>AddAsset("img.abc123.png", label: "img.png")</c> makes <c>Assets["img.png"]</c> return <c>img.abc123.png</c>.
/// </remarks>
Copy link
Copy Markdown

@LasseHerget LasseHerget left a comment

Choose a reason for hiding this comment

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

Big thanks @linkdotnet ! I was literally going to look into that this morning! :D
This change should definately resolve #1847.

I just added a few minor suggestions on what I personally would've done differently.
Neither of those suggestions target the logic itself though. That part looks perfect. :)

Let me know if you need another review after applying a suggestion or sth.

Kind regards, and I am really looking forward to this. :D

/// <param name="label">The label of the asset, used as the lookup key by the <see cref="ResourceAssetCollection"/> indexer. Pass <see langword="null"/> to add the asset without a label.</param>
/// <param name="properties">Additional properties to associate with the asset.</param>
[SuppressMessage("Design", "CA1054:URI-like parameters should not be strings", Justification = "Using string to align with ResourceAsset")]
public void AddAsset(string url, string? label = null, params ResourceAssetProperty[] properties)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion

Consider using an IEnumerable for the properties to support all enumerations:

	public void AddAsset(string url, string? label = null, params IEnumerable<ResourceAssetProperty> properties)

That would require...

  1. Since an IEnumerable does not have a length property, we'd need to initialize the props direct with the items:

    var props = new List<ResourceAssetProperty>(properties);

    or

    var props = properties.ToList();
  2. To avoid duplicate enumeration, we'd have to adjust the check for duplicate label property:

    if (props.Any(static property => string.Equals(property.Name, "label", StringComparison.Ordinal)))
  3. We'd have to consolidate the call from the BunitContext.

throw new ArgumentException("The label must not be empty or whitespace.", nameof(label));
}

if (properties.Any(static property => string.Equals(property.Name, "label", StringComparison.Ordinal)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider declaring a constant for the label property.

throw new ArgumentException("The label property is reserved when a label is provided.", nameof(properties));
}

props.Add(new ResourceAssetProperty("label", label));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Personal preference

IDE0090: Simplify new expression

Suggested change
props.Add(new ResourceAssetProperty("label", label));
props.Add(new("label", label));

Similar suggestion for line 127.

Comment thread src/bunit/BunitContext.cs

/// <summary>
/// Adds an asset to the <see cref="ResourceAssetCollection"/> that components
/// rendered with this <see cref="BunitContext"/> can access through their <c>Assets</c> property.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider referencing the assets property instead of just mentioning it:

Suggested change
/// rendered with this <see cref="BunitContext"/> can access through their <c>Assets</c> property.
/// rendered with this <see cref="BunitContext"/> can access through their <see cref="ComponentBase.Assets"/> property.

Comment thread src/bunit/BunitContext.cs
/// Adds an asset to the <see cref="ResourceAssetCollection"/> that components
/// rendered with this <see cref="BunitContext"/> can access through their <c>Assets</c> property.
/// </summary>
/// <remarks>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider inheriting the documentation (arguments and remarks) from the BunitRenderer, to avoid redundancy:

	/// <summary>
	/// Adds an asset to the <see cref="ResourceAssetCollection"/> that components
	/// rendered with this <see cref="BunitContext"/> can access through their <see cref="ComponentBase.Assets"/> property.
	/// </summary>
	/// <inheritdoc cref="BunitRenderer.AddAsset"/>

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.

Add support for Assets

3 participants