Skip to content

[ci] Enable testing building previous#33961

Open
rmarinho wants to merge 1 commit intonet11.0from
enable-back-previous-tests
Open

[ci] Enable testing building previous#33961
rmarinho wants to merge 1 commit intonet11.0from
enable-back-previous-tests

Conversation

@rmarinho
Copy link
Copy Markdown
Member

@rmarinho rmarinho commented Feb 9, 2026

Description of Change

This pull request re-enables and adds several integration tests for .NET MAUI templates targeting the "DotNetPrevious" framework across iOS, macOS, and Windows, and updates the CI pipeline to run these tests in parallel for both ARM64 and x64 architectures. These changes help ensure better test coverage for previous .NET versions and improve CI reliability.

Test Coverage Improvements:

  • Re-enabled previously commented-out [InlineData] test cases for "DotNetPrevious" in SimpleTemplateTest.cs, covering maui, maui-blazor, and mauilib templates for build, special character, and bad image file scenarios. [1] [2] [3]
  • Re-enabled and added [InlineData] test cases for "DotNetPrevious" in WindowsTemplateTest.cs, covering both packaged and unpackaged publish scenarios for maui and maui-blazor templates. [1] [2]
  • Added dedicated test methods in AppleTemplateTests.cs for running iOS tests on the "DotNetPrevious" framework, enabling parallel execution in CI.

CI Pipeline Enhancements:

  • Added new jobs to the ci.yml pipeline to run iOS template tests for "DotNetPrevious" on both ARM64 (mac_runios_previous_arm64) and x64 (mac_runios_previous) architectures, supporting both public and internal MacOS pools and improving CI coverage for previous .NET versions. [1] [2]

Copilot AI review requested due to automatic review settings February 9, 2026 12:17
@rmarinho rmarinho added area-testing Unit tests, device tests testing-infrastructure Issue relating to testing infrastructure testing-reenable copilot labels Feb 9, 2026
Copy link
Copy Markdown
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

Re-enables .NET MAUI template integration tests against DotNetPrevious and extends CI to run the iOS “previous” lane on both ARM64 and x64 macOS pools.

Changes:

  • Re-enabled DotNetPrevious [InlineData] cases in SimpleTemplateTest and WindowsTemplateTest.
  • Added an iOS test entrypoint (RunOniOS_Previous) in AppleTemplateTests for CI to target.
  • Added two new CI jobs to run RunOniOS_Previous on ARM64 and x64 macOS pools.

Reviewed changes

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

File Description
src/TestUtils/src/Microsoft.Maui.IntegrationTests/WindowsTemplateTest.cs Re-enables Windows publish tests for DotNetPrevious.
src/TestUtils/src/Microsoft.Maui.IntegrationTests/SimpleTemplateTest.cs Re-enables template build/pack scenarios for DotNetPrevious.
src/TestUtils/src/Microsoft.Maui.IntegrationTests/AppleTemplateTests.cs Adds RunOniOS_Previous test method for CI targeting.
eng/pipelines/ci.yml Adds new macOS jobs to run iOS previous tests on ARM64 and x64.

@@ -62,11 +62,14 @@ public AppleTemplateTests(IntegrationTestFixture fixture, ITestOutputHelper outp
if (true) return; // Skip: "Running Apple templates is only supported on macOS."
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

if (true) return; disables the intended platform guard and does not actually skip tests (it just exits the constructor early). This can cause Apple tests to run on non-macOS environments (or proceed without proper setup) instead of being skipped. Replace this with the standard macOS guard used in the repo (e.g., if (!TestEnvironment.IsMacOS) Assert.Ignore("Running Apple templates is only supported on macOS.");) rather than returning unconditionally.

Copilot uses AI. Check for mistakes.
Comment on lines +65 to 73
// DotNetPrevious test methods
[Theory]
[InlineData("maui", "Debug", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui", "Release", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui-blazor", "Debug", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui-blazor", "Release", DotNetPrevious, RuntimeVariant.Mono, null)]
public void RunOniOS_Previous(string id, string config, string framework, RuntimeVariant runtimeVariant, string? trimMode)
=> RunOniOS(id, config, framework, runtimeVariant, trimMode);

Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The new RunOniOS_Previous is a single [Theory] with multiple [InlineData] cases, but the comment immediately below states the CI model relies on “individual test methods” so each configuration can run in a separate job. With the current structure, the testName: RunOniOS_Previous job will execute all four cases serially in one job per arch. Consider splitting these into separate [Test] methods (or separate theory methods) per configuration (e.g., maui Debug, maui Release, maui-blazor Debug, maui-blazor Release) so the code matches the comment/CI strategy and keeps job runtimes predictable.

Suggested change
// DotNetPrevious test methods
[Theory]
[InlineData("maui", "Debug", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui", "Release", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui-blazor", "Debug", DotNetPrevious, RuntimeVariant.Mono, null)]
[InlineData("maui-blazor", "Release", DotNetPrevious, RuntimeVariant.Mono, null)]
public void RunOniOS_Previous(string id, string config, string framework, RuntimeVariant runtimeVariant, string? trimMode)
=> RunOniOS(id, config, framework, runtimeVariant, trimMode);
// DotNetPrevious test methods (individual methods per configuration for CI filtering)
[Fact]
public void RunOniOS_Previous_MauiDebug() => RunOniOS("maui", "Debug", DotNetPrevious, RuntimeVariant.Mono, null);
[Fact]
public void RunOniOS_Previous_MauiRelease() => RunOniOS("maui", "Release", DotNetPrevious, RuntimeVariant.Mono, null);
[Fact]
public void RunOniOS_Previous_BlazorDebug() => RunOniOS("maui-blazor", "Debug", DotNetPrevious, RuntimeVariant.Mono, null);
[Fact]
public void RunOniOS_Previous_BlazorRelease() => RunOniOS("maui-blazor", "Release", DotNetPrevious, RuntimeVariant.Mono, null);

Copilot uses AI. Check for mistakes.
Comment thread eng/pipelines/ci.yml
- name: mac_runios_previous_arm64
${{ if eq(variables['Build.DefinitionName'], 'maui-pr') }}:
pool: ${{ parameters.MacOSPool.public }}
${{ else }}:
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

There are trailing spaces after ${{ else }}:. This is minor, but it can trip YAML linting or style checks. Consider removing the extra whitespace.

Copilot uses AI. Check for mistakes.
Comment thread eng/pipelines/ci.yml
- name: mac_runios_previous
${{ if eq(variables['Build.DefinitionName'], 'maui-pr') }}:
pool: ${{ parameters.MacOSPool.public }}
${{ else }}:
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

There are trailing spaces after ${{ else }}:. This is minor, but it can trip YAML linting or style checks. Consider removing the extra whitespace.

Copilot uses AI. Check for mistakes.
@rmarinho
Copy link
Copy Markdown
Member Author

/rebase

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 17, 2026

🟡 .NET MAUI Code Review — Changes Suggested

👋 @rmarinho — new code review results are available. Please review the latest session below.

🟡 Review Session — Changes Suggestede8e499b · [ci] Enable testing building previous · 2026-04-17 09:11 UTC

Independent Assessment

What this changes:

  1. Re-enables commented-out [InlineData] cases for DotNetPrevious (net10.0) in SimpleTemplateTest.cs (Build, BuildWithMauiVersion, PackCoreLib) and WindowsTemplateTest.cs (PublishPackaged, PublishUnpackaged).
  2. Adds a new RunOniOS_Previous [Theory] in AppleTemplateTests.cs delegating to the private RunOniOS helper.
  3. Adds two new CI job pairs in ci.yml (mac_runios_previous_arm64, mac_runios_previous) to run that theory in both pool flavors.

Inferred motivation: Restore coverage that was disabled during the net11.0 branch bring-up, now that net10.0 is stable enough to build as the "previous" target.

Reconciliation with PR Narrative

Author's description matches the code. One wording note: the description says the AppleTemplateTests change enables "parallel execution in CI", but the implementation uses a single [Theory] method that CI targets with --filter "Name=RunOniOS_Previous", which runs all four InlineData cases serially in one job. The implementation doesn't actually deliver the claimed parallel fan-out.

Findings

❌ Error — Required CI check is red

maui-pr (Run Integration Tests RunOniOS_Previous ARM64) failed in ~8 min (no TRX output, so an early failure, not a timeout). The WindowsTemplates windows and AOT macOS integration lanes are also red. Since the whole purpose of this PR is to re-enable these tests, a red result on the exact lanes being re-enabled is a hard blocker. This needs to be investigated and either fixed, or the problematic InlineData rows must stay commented out with an issue link. Logs from build 1350355 have been purged — kicking a fresh run is the next step to capture the failure.

⚠️ Warning — [Theory] pattern contradicts the stated parallelism design

AppleTemplateTests.cs lines 71-72 explicitly document the design:

// Individual test methods for each configuration to enable parallel CI runs
// CI uses --filter "Name=TestMethodName" to run each test in a separate job

All existing DotNetCurrent entries follow that rule with four [Fact] wrappers (RunOniOS_MauiDebug, RunOniOS_MauiRelease, RunOniOS_BlazorDebug, RunOniOS_BlazorRelease). The new RunOniOS_Previous bundles four configurations into a single [Theory], and CI adds only one job pair that matches it — so Previous coverage runs ~4× the work serially in a lane with the same 45-min timeout.

Suggested fix: mirror the existing pattern with four [Fact] wrappers (e.g. RunOniOS_MauiDebug_Previous, …) and four CI job pairs. It also makes failure attribution per-config much clearer, which matters when reactivating long-disabled tests.

💡 Suggestion — Remove the now-stale commented block

AppleTemplateTests.cs:65-69 still has the old // [InlineData(...)] lines that this PR supersedes. Delete them to avoid confusion.

💡 Suggestion — Trailing whitespace in YAML

The new blocks in eng/pipelines/ci.yml have ${{ else }}: with trailing spaces. Minor, but the file is otherwise clean.

Devil's Advocate

  • Could [Theory] be intentional to minimize CI cost? Possibly, but then the PR description shouldn't claim "parallel execution" — the two statements are inconsistent. Either update the description or change the implementation.
  • Could the red checks be a flaky pre-existing lane? WindowsTemplates windows and AOT macOS sometimes flake, but RunOniOS_Previous ARM64 is a lane this PR just introduced — its failure is by definition caused by this PR.
  • Is re-enabling just "uncommenting" low-risk? Normally yes, but the red CI proves that at least some of the re-enabled configurations don't build/run cleanly with the current net10.0 SDK the CI image provides. Merging as-is would leave required checks red.

Verdict: NEEDS_CHANGES

Confidence: high
Summary: Mechanically straightforward PR, but (1) the CI lane this PR adds is failing, and (2) the new Apple iOS test uses a single [Theory] while the file's documented pattern — and all other Current-framework tests — use per-configuration [Fact] methods for parallel CI. Fix the failing lane (or scope down the re-enabled InlineData rows), and split the Apple test into four [Fact] wrappers with matching CI jobs to match the existing design.


Copy link
Copy Markdown
Contributor

@kubaflo kubaflo left a comment

Choose a reason for hiding this comment

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

@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Apr 30, 2026

⚠️ Merge Conflict Detected — This PR has merge conflicts with its target branch. Please rebase onto the target branch and resolve the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-testing Unit tests, device tests copilot testing-infrastructure Issue relating to testing infrastructure testing-reenable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants