Skip to content

Conversation

@maraf
Copy link
Member

@maraf maraf commented Apr 24, 2025

OverrideHtmlAssetPlaceholders is now default for both wasm & blazor apps

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm labels Apr 24, 2025
@maraf maraf added this to the 10.0.0 milestone Apr 24, 2025
@maraf maraf self-assigned this Apr 24, 2025
@maraf maraf marked this pull request as ready for review April 25, 2025 07:05
Copilot AI review requested due to automatic review settings April 25, 2025 07:05
@maraf maraf requested a review from ilonatommy as a code owner April 25, 2025 07:05
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 cleans up the Wasm.Build.Tests by removing obsolete tests and parameters now that OverrideHtmlAssetPlaceholders is enabled by default for both wasm and blazor apps.

  • Removed the explicit OverrideHtmlAssetPlaceholders parameter in tests and updated AssertAppBundle values in PreloadingTests.
  • Deleted the FingerprintingTests file and the test for OverrideHtmlAssetPlaceholders in Blazor MiscTests.
  • Updated project and job list files to reflect the consolidated testing strategy.

Reviewed Changes

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

Show a summary per file
File Description
src/mono/wasm/Wasm.Build.Tests/PreloadingTests.cs Removed obsolete MSBuild flag and updated AssertAppBundle parameter.
src/mono/wasm/Wasm.Build.Tests/FingerprintingTests.cs Entire file removed as the behavior is now covered elsewhere.
src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests.cs Removed outdated test for OverrideHtmlAssetPlaceholders from Blazor tests.
src/mono/wasm/Wasm.Build.Tests/Blazor/AppsettingsTests.cs Eliminated the extra property for OverrideHtmlAssetPlaceholders.
eng/testing/scenarios/BuildWasmAppsJobsList.txt Removed the FingerprintingTests entry from the job list.
Comments suppressed due to low confidence (3)

src/mono/wasm/Wasm.Build.Tests/FingerprintingTests.cs:1

  • The removal of the FingerprintingTests file may reduce test coverage for fingerprinting behavior. Ensure that the scenarios previously covered in this file are adequately tested in the remaining test suites.
// Licensed to the .NET Foundation under one or more agreements.

src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests.cs:98

  • The removal of the TestOverrideHtmlAssetPlaceholders in Blazor tests may leave a gap in verifying the behavior of OverrideHtmlAssetPlaceholders. Verify that this scenario is covered elsewhere in the test suite.
public async Task TestOverrideHtmlAssetPlaceholders() { ... }

eng/testing/scenarios/BuildWasmAppsJobsList.txt:48

  • Removing the FingerprintingTests job from the testing scenarios can reduce automated verification of fingerprinting behavior. Confirm that all important fingerprinting scenarios are still being exercised by other tests.
Wasm.Build.Tests.FingerprintingTests

@ilonatommy
Copy link
Member

checking if #114548 fixed the

PreAllocatedOverlapped.Portable.cs(10,58): error CS0535: (NETCORE_ENGINEERING_TELEMETRY=Build) 'PreAllocatedOverlapped' does not implement interface member 'IDisposable.Dispose()'

failures

@maraf maraf merged commit 98972b5 into dotnet:main Apr 30, 2025
24 of 26 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants