Skip to content

[StaticWebAssets] Move static web assets tests into their own project #49645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Jul 3, 2025

  • I asked copilot to move the StaticWebAssets tests to their own project since they were on the .Razor.Tests project
  • Confirmation of tests running on CI
    image

@javiercn javiercn marked this pull request as ready for review July 3, 2025 15:21
@Copilot Copilot AI review requested due to automatic review settings July 3, 2025 15:21
@javiercn javiercn requested review from a team as code owners July 3, 2025 15:21
Copy link
Contributor

@Copilot 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 moves the Static Web Assets tests out of the Razor test project into their own dedicated test project and updates all related namespaces, references, and resources.

  • Created a new Microsoft.NET.Sdk.StaticWebAssets.Tests project with its own .csproj, embedded resources, and targets file.
  • Updated namespaces and manifest resource paths in existing test files to point to the new project.
  • Adjusted solution, project references, and added InternalsVisibleTo to wire up the new test project.

Reviewed Changes

Copilot reviewed 50 out of 155 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/content/ExternalStaticAssets.targets New MSBuild targets file defining external static assets logic
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetsIntegrationTest.cs Refactored namespace, manifest stream path, and SDK handling
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/Microsoft.NET.Sdk.StaticWebAssets.Tests.csproj New test project setup, dependencies, and embedded resources
src/StaticWebAssetsSdk/Tasks/Microsoft.NET.Sdk.StaticWebAssets.Tasks.csproj Added InternalsVisibleTo for the new test project
sdk.slnx Included the new test project in the solution
Comments suppressed due to low confidence (4)

test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetsFingerprintingTest.cs:12

  • The file name (StaticWebAssetsFingerprintingTest.cs) does not match the class name (StaticWebAssetsContentFingerprintingIntegrationTest). Consider renaming the file to StaticWebAssetsContentFingerprintingIntegrationTest.cs or adjusting the class name to align with the file.
public class StaticWebAssetsContentFingerprintingIntegrationTest(ITestOutputHelper log) : AspNetSdkBaselineTest(log)

test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/ApplyCssScopesTest.cs:11

  • The file is named ApplyCssScopesTest.cs but the class is named ApplyAllCssScopesTest. Rename one or the other to keep file and class names consistent.
    public class ApplyAllCssScopesTest

test/Microsoft.NET.Sdk.StaticWebAssets.Tests/ScopedCssIntegrationTests.cs:11

  • The file is named ScopedCssIntegrationTests.cs (plural) but the class is ScopedCssIntegrationTest (singular). Align the file name and class name for clarity.
    public class ScopedCssIntegrationTest(ITestOutputHelper log)

test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetsIntegrationTest.cs:478

  • Add a test case to cover the branch where the project file has no Sdk attribute, ensuring the null branch and attribute insertion logic are exercised.
                        if (sdkAttribute == null)

@javiercn javiercn requested a review from MackinnonBuck July 3, 2025 18:41
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.

2 participants