Merge branch 'v17/dev'
This commit is contained in:
@@ -0,0 +1,135 @@
|
||||
---
|
||||
name: umb-release-notes
|
||||
description: Improve a set of auto-generated GitHub release notes for an Umbraco CMS release. Cross-checks the notes against every PR carrying the release label, adds any that are missing, re-files every PR under the most appropriate category, and strips purely-internal entries. Use whenever the user asks to tidy up, improve, complete, or recategorize release notes for a given version, or mentions a release-notes text file plus a version number.
|
||||
argument-hint: <version> <path-to-generated-notes-file>
|
||||
---
|
||||
|
||||
# Umbraco CMS - Improve Release Notes
|
||||
|
||||
Takes a file of auto-generated GitHub release notes and produces an improved version that:
|
||||
|
||||
1. **Is complete** — every merged PR carrying the `release/<version>` label appears.
|
||||
2. **Is well-categorized** — every PR sits under the most appropriate heading.
|
||||
3. **Is free of noise** — purely-internal entries of no value to a reader are removed.
|
||||
|
||||
The result is written to a **new** file alongside the input, so the user can diff the two.
|
||||
|
||||
**Run autonomously.** Do NOT use `AskUserQuestion` once the required arguments (version and input file path) are available — only ask if one of them is missing from `$ARGUMENTS` and cannot be inferred (see Arguments). Beyond that, make the categorization calls yourself using the rules below; if a handful are genuinely borderline, place them anyway and note the borderline ones in your closing summary so the user can override.
|
||||
|
||||
## Arguments
|
||||
|
||||
`$ARGUMENTS` contains two values:
|
||||
|
||||
1. **Version** — e.g. `17.5.0`, `18.1.0`. The GitHub label to search is `release/<version>` (so version `17.5.0` → label `release/17.5.0`).
|
||||
2. **Input file path** — full path to the text file holding the auto-generated notes (e.g. `C:\Temp\release-17.5.0-rc.md`).
|
||||
|
||||
If either is missing, ask the user once for the missing value, then proceed.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
Run `gh auth status`. If it fails, tell the user to authenticate `gh` (e.g. `gh auth login`) and stop — the skill needs the GitHub CLI to query PRs. The repo is always `umbraco/Umbraco-CMS`.
|
||||
|
||||
## Procedure
|
||||
|
||||
### 1. Read the input notes
|
||||
|
||||
Read the input file. Note its structure — it is GitHub's generated format:
|
||||
|
||||
- A leading HTML comment (`<!-- Release notes generated ... -->`).
|
||||
- A `## What's Changed` heading followed by `### <emoji> <Category>` sub-headings, each with `* <title> by @<author> in <url>` bullets.
|
||||
- A trailing `## New Contributors` section and a `**Full Changelog**: ...` line.
|
||||
|
||||
Extract the set of PR numbers already present (parse the `/pull/<number>` from each bullet). Preserve each existing bullet's **exact text** (title, author, URL) when you re-emit it — only its category placement may change.
|
||||
|
||||
### 2. Fetch every labelled PR
|
||||
|
||||
```bash
|
||||
gh pr list --repo umbraco/Umbraco-CMS --label "release/<version>" --state closed --limit 1000 \
|
||||
--json number,title,author,labels,mergedAt \
|
||||
--jq '.[] | select(.mergedAt != null) | "\(.number)\t\(.author.login)\t\([.labels[].name] | join(", "))\t\(.title)"' | sort -n
|
||||
```
|
||||
|
||||
This is the authoritative list of what the release *should* contain. Each row gives number, author, labels, title.
|
||||
|
||||
**Guard against silent truncation.** `gh pr list` caps at `--limit` without warning, so a large release could drop the overflow and the skill would still look "complete". Count the returned rows and compare against the limit:
|
||||
|
||||
```bash
|
||||
gh pr list --repo umbraco/Umbraco-CMS --label "release/<version>" --state closed --limit 1000 --json number --jq 'length'
|
||||
```
|
||||
|
||||
If this equals 1000, the limit was hit — raise `--limit` and re-fetch before continuing. Do **not** proceed on a truncated list.
|
||||
|
||||
### 3. Reconcile
|
||||
|
||||
- **Missing labelled PRs** (labelled but not in the input file): these must be **added**. Build a bullet as `* <title> by @<author> in https://github.com/umbraco/Umbraco-CMS/pull/<number>`.
|
||||
- **Author handle.** `<author>` in the template is the raw `.author.login` value — the bullet supplies the leading `@`, so do not prepend another. `gh`'s `.author.login` already returns bot accounts with the `[bot]` suffix as part of the login — Dependabot comes back as `dependabot[bot]`, not `dependabot` or `app/dependabot` (the `app/` form only appears in git committer metadata and CODEOWNERS, never in `gh`'s JSON). So the login is already in the right shape; use it verbatim (e.g. `.author.login` of `dependabot[bot]` renders as `@dependabot[bot]`, matching what GitHub's generator wrote for the existing bullets). The only thing to guard against is accidentally stripping or altering the `[bot]` suffix.
|
||||
- **PRs in the file but not labelled**: keep them. The generated notes span a commit range (see the `Full Changelog` compare link), so they legitimately include backports / earlier-version PRs that lack the current label. For any of these you need to categorize, fetch its labels with:
|
||||
|
||||
```bash
|
||||
gh pr view <number> --repo umbraco/Umbraco-CMS --json number,title,labels \
|
||||
--jq '"\(.number)\t\([.labels[].name] | join(", "))\t\(.title)"'
|
||||
```
|
||||
|
||||
Do **not** invent or alter the `New Contributors` section — carry it over verbatim. You cannot reliably recompute first-time contributors, so leave it as the generator produced it (mention this in the summary).
|
||||
|
||||
### 4. Categorize every PR
|
||||
|
||||
Use exactly these headings, in this order. Omit any heading that ends up with no entries.
|
||||
|
||||
| Heading | What goes here | Primary signal |
|
||||
|---|---|---|
|
||||
| `### 🙌 Notable Changes` | **Don't recategorize existing entries.** Label-driven — but still add any missing PR carrying this label here. | label `category/notable` |
|
||||
| `### 💥 Breaking Changes` | **Don't recategorize existing entries.** Label-driven — but still add any missing PR carrying this label here. | label `category/breaking` |
|
||||
| `### 📦 Dependencies` | Dependency bumps | label `dependencies`; or dependabot author |
|
||||
| `### 🚀 New Features` | New user- or developer-facing capability | label `type/feature` / `category/feature`; or title introduces/adds a genuinely new capability |
|
||||
| `### 🚤 Performance` | Performance improvements | label `category/performance`; or `Performance:` title prefix |
|
||||
| `### 🌈 Accessibility Improvements` | A11y improvements (labels, contrast, keyboard) | label `category/accessibility` / `accessibility`; or clear a11y intent (e.g. "improve contrast", "missing labels") |
|
||||
| `### 🐛 Bug Fixes` | Fixes to broken/incorrect behaviour | default for anything describing a fix |
|
||||
| `### 🧪 Testing` | Test additions/changes only | label `category/test-automation` / `area/test`; or `E2E`/`QA`/"acceptance tests"/"unit test coverage"/"add tests" titles |
|
||||
| `### 🛡️ Code Quality, Documentation and Refactoring` | Refactors, deprecations, API tidy-ups, XML/MD documentation, knowledge-base (`MD`) updates | label `category/refactor`; or titles about refactoring, deprecating, renaming, documenting, constants extraction, MD/CLAUDE.md content |
|
||||
| `### 🧑💻 Developer Experience` | Things that improve the experience of developers building on or contributing to Umbraco — dev tooling, build/watch ergonomics, test mocks/harnesses, backoffice dev utilities | `Developer Experience` title prefix; dev tooling; mock/harness changes |
|
||||
|
||||
**Rules:**
|
||||
|
||||
- **Notable and Breaking are off-limits for recategorization** — never move a PR that is *already in the input file* into or out of these sections; they are driven purely by their labels and the generator placed them correctly. This does **not** exempt them from completeness: a PR discovered as missing in step 3 that carries `category/notable` or `category/breaking` must still be **added** under the matching section.
|
||||
- Label signals beat title wording, except a `Performance:`/`Developer Experience:` title prefix is decisive for its section.
|
||||
- A PR with both `type/feature` and `category/refactor` whose title clearly describes a refactor (e.g. "swap relative imports", "re-export type") belongs under Code Quality, not New Features.
|
||||
- "Add ... tests"/"unit test coverage" → Testing, even if it also touches docs. If a PR adds XML documentation *and* tests, lead with where the title's emphasis lies (documentation → Code Quality; test coverage → Testing).
|
||||
- When a PR is genuinely 50/50, pick the more reader-useful heading and list it in your closing summary as borderline.
|
||||
|
||||
### 5. Remove purely-internal noise
|
||||
|
||||
Drop entries that have **no value to anyone reading release notes** — pure repository plumbing with no shipped impact. Examples:
|
||||
|
||||
- Branch/merge maintenance ("Fix main branch after merge issue").
|
||||
- CI/pipeline fixes that don't change the product.
|
||||
- Reverts of changes that never shipped in a release.
|
||||
|
||||
**Keep** anything that ships in the product or genuinely helps developers building on Umbraco — that includes documentation/MD updates, dev tooling, and test mocks (those go to Code Quality or Developer Experience, they are *not* noise). When unsure whether something is noise, keep it and flag it in the summary rather than silently dropping it. List every removal in your closing summary.
|
||||
|
||||
### 6. Write the output
|
||||
|
||||
Write to a new file in the **same folder** as the input, named by appending ` - with updates` before the extension:
|
||||
|
||||
- Input `C:\Temp\release-17.5.0-rc.md` → Output `C:\Temp\release-17.5.0-rc - with updates.md`
|
||||
|
||||
Preserve the leading HTML comment, the `## What's Changed` heading, the `## New Contributors` section, and the `**Full Changelog**` line exactly. Only the `### <category>` groupings and their bullets change.
|
||||
|
||||
### 7. Report
|
||||
|
||||
Give a concise summary:
|
||||
|
||||
- Count of PRs added (with their numbers), and which categories they landed in.
|
||||
- Notable recategorizations (PRs moved out of the catch-all Bug Fixes into Features/Performance/Testing/etc.).
|
||||
- Every entry removed, with the one-line reason.
|
||||
- Any borderline calls the user may want to override.
|
||||
- The output file path.
|
||||
|
||||
## Verification
|
||||
|
||||
Before reporting done, confirm:
|
||||
|
||||
- Every PR number from step 2 is present in the output (except any you deliberately removed in step 5 — and those must be in the removal list).
|
||||
- No PR appears under more than one heading.
|
||||
- Notable and Breaking sections are byte-for-byte unchanged from the input.
|
||||
- The header comment, New Contributors, and Full Changelog lines are intact.
|
||||
@@ -57,7 +57,7 @@
|
||||
<PackageVersion Include="MailKit" Version="4.16.0" />
|
||||
<PackageVersion Include="Markdig" Version="1.1.3" />
|
||||
<PackageVersion Include="Markdown" Version="2.2.1" />
|
||||
<PackageVersion Include="MessagePack" Version="3.1.4" />
|
||||
<PackageVersion Include="MessagePack" Version="3.1.7" />
|
||||
<PackageVersion Include="MiniProfiler.AspNetCore.Mvc" Version="4.5.4" />
|
||||
<PackageVersion Include="MiniProfiler.Shared" Version="4.5.4" />
|
||||
<PackageVersion Include="ncrontab" Version="3.4.0" />
|
||||
|
||||
@@ -107,12 +107,15 @@ public class PackageMigrationRunner
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Runs the all specified package migration plans and publishes a <see cref="MigrationPlansExecutedNotification" />
|
||||
/// if all are successful.
|
||||
/// Runs all the specified package migration plans and publishes a <see cref="MigrationPlansExecutedNotification" />.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// All plans are run to completion even if one fails, so that one package's failure does not block another's.
|
||||
/// A failed plan is reported via <see cref="ExecutedMigrationPlan.Successful" /> on the returned result rather
|
||||
/// than by throwing; callers must inspect the results to detect a failure.
|
||||
/// </remarks>
|
||||
/// <param name="plansToRun"></param>
|
||||
/// <returns></returns>
|
||||
/// <exception cref="Exception">If any plan fails it will throw an exception.</exception>
|
||||
public async Task<IEnumerable<ExecutedMigrationPlan>> RunPackagePlansAsync(IEnumerable<string> plansToRun)
|
||||
{
|
||||
List<ExecutedMigrationPlan> results = new();
|
||||
|
||||
@@ -11,6 +11,7 @@ using Umbraco.Cms.Core.Exceptions;
|
||||
using Umbraco.Cms.Core.Logging;
|
||||
using Umbraco.Cms.Core.Notifications;
|
||||
using Umbraco.Cms.Core.Services;
|
||||
using Umbraco.Cms.Infrastructure.Migrations;
|
||||
using Umbraco.Cms.Infrastructure.Migrations.Install;
|
||||
using Umbraco.Cms.Infrastructure.Migrations.Upgrade;
|
||||
using Umbraco.Cms.Infrastructure.Runtime;
|
||||
@@ -163,7 +164,23 @@ public class UnattendedUpgrader : INotificationAsyncHandler<RuntimeUnattendedUpg
|
||||
|
||||
try
|
||||
{
|
||||
await _packageMigrationRunner.RunPackagePlansAsync(pendingMigrations);
|
||||
IEnumerable<ExecutedMigrationPlan> executedPlans =
|
||||
await _packageMigrationRunner.RunPackagePlansAsync(pendingMigrations);
|
||||
|
||||
// Failed plans are reported via the result, not by throwing (the runner deliberately runs all plans to
|
||||
// completion so one package's failure doesn't block another's). Surface them as a boot failure here so the
|
||||
// failure is observable, mirroring the core upgrade path - otherwise the migration stays pending and the
|
||||
// runtime re-derives Upgrading on every boot, leaving the site stuck on the maintenance page.
|
||||
// All failures are reported together.
|
||||
var failedPlans = executedPlans.Where(plan => plan.Successful is false).ToList();
|
||||
if (failedPlans.Count > 0)
|
||||
{
|
||||
SetRuntimeError(CreatePackageMigrationError(failedPlans));
|
||||
notification.UnattendedUpgradeResult =
|
||||
RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors;
|
||||
return;
|
||||
}
|
||||
|
||||
notification.UnattendedUpgradeResult = RuntimeUnattendedUpgradeNotification.UpgradeResult.PackageMigrationComplete;
|
||||
|
||||
// Migration plans may have changed published content, so refresh the distributed cache to ensure consistency on first request.
|
||||
@@ -200,6 +217,22 @@ public class UnattendedUpgrader : INotificationAsyncHandler<RuntimeUnattendedUpg
|
||||
}
|
||||
}
|
||||
|
||||
private static Exception CreatePackageMigrationError(IReadOnlyList<ExecutedMigrationPlan> failedPlans)
|
||||
{
|
||||
static Exception ToException(ExecutedMigrationPlan plan)
|
||||
=> plan.Exception ?? new UnattendedInstallException(
|
||||
$"An error occurred while running the unattended package migration '{plan.Plan.Name}'.");
|
||||
|
||||
if (failedPlans.Count == 1)
|
||||
{
|
||||
return ToException(failedPlans[0]);
|
||||
}
|
||||
|
||||
return new AggregateException(
|
||||
$"{failedPlans.Count} unattended package migrations failed: {string.Join(", ", failedPlans.Select(plan => plan.Plan.Name))}.",
|
||||
failedPlans.Select(ToException));
|
||||
}
|
||||
|
||||
private void SetRuntimeError(Exception exception)
|
||||
=> _runtimeState.Configure(
|
||||
RuntimeLevel.BootFailed,
|
||||
|
||||
+115
@@ -0,0 +1,115 @@
|
||||
// Copyright (c) Umbraco.
|
||||
// See LICENSE for more details.
|
||||
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using Moq;
|
||||
using NUnit.Framework;
|
||||
using Umbraco.Cms.Core;
|
||||
using Umbraco.Cms.Core.Cache;
|
||||
using Umbraco.Cms.Core.Configuration;
|
||||
using Umbraco.Cms.Core.Configuration.Models;
|
||||
using Umbraco.Cms.Core.Events;
|
||||
using Umbraco.Cms.Core.Logging;
|
||||
using Umbraco.Cms.Core.Migrations;
|
||||
using Umbraco.Cms.Core.Notifications;
|
||||
using Umbraco.Cms.Core.Packaging;
|
||||
using Umbraco.Cms.Core.Scoping;
|
||||
using Umbraco.Cms.Core.Services;
|
||||
using Umbraco.Cms.Infrastructure.Install;
|
||||
using Umbraco.Cms.Infrastructure.Migrations;
|
||||
using Umbraco.Cms.Infrastructure.Migrations.Install;
|
||||
using Umbraco.Cms.Infrastructure.Runtime;
|
||||
using Umbraco.Cms.Tests.Common.Testing;
|
||||
using Umbraco.Cms.Tests.Integration.Testing;
|
||||
|
||||
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Install;
|
||||
|
||||
[TestFixture]
|
||||
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
|
||||
internal sealed class UnattendedPackageMigrationTests : UmbracoIntegrationTest
|
||||
{
|
||||
// Exercises the real MigrationPlanExecutor, which catches a failing migration step and returns a
|
||||
// result with Successful = false rather than throwing. This test locks in that this silently-failed
|
||||
// package migration is converted into a BootFailed state (and not left stuck on Upgrading), and that
|
||||
// the original migration exception is surfaced - the full chain the unit tests mock away.
|
||||
[Test]
|
||||
public async Task HandleAsync_WhenRealPackageMigrationThrows_SetsBootFailedAndSurfacesTheException()
|
||||
{
|
||||
var plan = new ThrowingPackageMigrationPlan();
|
||||
var planCollection = new PackageMigrationPlanCollection(() => new PackageMigrationPlan[] { plan });
|
||||
|
||||
var packageMigrationRunner = new PackageMigrationRunner(
|
||||
GetRequiredService<IProfilingLogger>(),
|
||||
GetRequiredService<ICoreScopeProvider>(),
|
||||
new PendingPackageMigrations(NullLogger<PendingPackageMigrations>.Instance, planCollection),
|
||||
planCollection,
|
||||
GetRequiredService<IMigrationPlanExecutor>(), // the real executor that swallows the failure
|
||||
GetRequiredService<IKeyValueService>(),
|
||||
GetRequiredService<IEventAggregator>(),
|
||||
NullLogger<PackageMigrationRunner>.Instance);
|
||||
|
||||
Exception? capturedError = null;
|
||||
var runtimeState = new Mock<IRuntimeState>();
|
||||
runtimeState.SetupGet(x => x.Level).Returns(RuntimeLevel.Upgrading);
|
||||
runtimeState.SetupGet(x => x.Reason).Returns(RuntimeLevelReason.UpgradePackageMigrations);
|
||||
runtimeState.SetupGet(x => x.StartupState).Returns(new Dictionary<string, object>
|
||||
{
|
||||
[RuntimeState.PendingPackageMigrationsStateKey] = (IReadOnlyList<string>)new[] { ThrowingPackageMigrationPlan.PlanName },
|
||||
});
|
||||
runtimeState
|
||||
.Setup(x => x.Configure(RuntimeLevel.BootFailed, It.IsAny<RuntimeLevelReason>(), It.IsAny<Exception?>()))
|
||||
.Callback<RuntimeLevel, RuntimeLevelReason, Exception?>((_, _, exception) => capturedError = exception);
|
||||
|
||||
var upgrader = new UnattendedUpgrader(
|
||||
GetRequiredService<IProfilingLogger>(),
|
||||
GetRequiredService<IUmbracoVersion>(),
|
||||
GetRequiredService<DatabaseBuilder>(),
|
||||
runtimeState.Object,
|
||||
packageMigrationRunner,
|
||||
Options.Create(new UnattendedSettings()),
|
||||
GetRequiredService<DistributedCache>(),
|
||||
NullLogger<UnattendedUpgrader>.Instance);
|
||||
|
||||
var notification = new RuntimeUnattendedUpgradeNotification();
|
||||
|
||||
// The failure must not propagate as a thrown exception - it has to be reported as a boot failure.
|
||||
await upgrader.HandleAsync(notification, CancellationToken.None);
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(notification.UnattendedUpgradeResult, Is.EqualTo(RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors));
|
||||
runtimeState.Verify(
|
||||
x => x.Configure(RuntimeLevel.BootFailed, RuntimeLevelReason.BootFailedOnException, It.IsNotNull<Exception>()),
|
||||
Times.Once);
|
||||
Assert.That(capturedError, Is.Not.Null);
|
||||
Assert.That(capturedError?.ToString(), Does.Contain(ThrowingMigration.ExceptionMessage));
|
||||
});
|
||||
}
|
||||
|
||||
// Nested + private so the TypeFinder (which excludes nested-private types) does not auto-discover this
|
||||
// PackageMigrationPlan and run it during every other integration test that boots a full server.
|
||||
private sealed class ThrowingPackageMigrationPlan : PackageMigrationPlan
|
||||
{
|
||||
public const string PlanName = "IntegrationThrowingPackage";
|
||||
|
||||
public ThrowingPackageMigrationPlan()
|
||||
: base(PlanName)
|
||||
{
|
||||
}
|
||||
|
||||
protected override void DefinePlan() => To<ThrowingMigration>("throw");
|
||||
}
|
||||
|
||||
private sealed class ThrowingMigration : AsyncMigrationBase
|
||||
{
|
||||
public const string ExceptionMessage = "Integration test package migration deliberately threw.";
|
||||
|
||||
public ThrowingMigration(IMigrationContext context)
|
||||
: base(context)
|
||||
{
|
||||
}
|
||||
|
||||
protected override Task MigrateAsync() => Task.FromException(new InvalidOperationException(ExceptionMessage));
|
||||
}
|
||||
}
|
||||
+244
@@ -0,0 +1,244 @@
|
||||
// Copyright (c) Umbraco.
|
||||
// See LICENSE for more details.
|
||||
|
||||
using System.Data;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using Moq;
|
||||
using NUnit.Framework;
|
||||
using Umbraco.Cms.Core;
|
||||
using Umbraco.Cms.Core.Cache;
|
||||
using Umbraco.Cms.Core.Configuration;
|
||||
using Umbraco.Cms.Core.Configuration.Models;
|
||||
using Umbraco.Cms.Core.Events;
|
||||
using Umbraco.Cms.Core.Exceptions;
|
||||
using Umbraco.Cms.Core.Logging;
|
||||
using Umbraco.Cms.Core.Migrations;
|
||||
using Umbraco.Cms.Core.Notifications;
|
||||
using Umbraco.Cms.Core.Packaging;
|
||||
using Umbraco.Cms.Core.Scoping;
|
||||
using Umbraco.Cms.Core.Services;
|
||||
using Umbraco.Cms.Core.Sync;
|
||||
using Umbraco.Cms.Infrastructure.Install;
|
||||
using Umbraco.Cms.Infrastructure.Migrations;
|
||||
using Umbraco.Cms.Infrastructure.Runtime;
|
||||
|
||||
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Install;
|
||||
|
||||
[TestFixture]
|
||||
public class UnattendedUpgraderTests
|
||||
{
|
||||
[Test]
|
||||
public async Task RunPackageMigrations_WhenSinglePlanFails_SetsBootFailedWithThePlanException()
|
||||
{
|
||||
var planException = new InvalidOperationException("migration step exploded");
|
||||
(UnattendedUpgrader sut, Mock<IRuntimeState> runtimeState) =
|
||||
CreateScenario(new PlanSpec("Failing Package", Successful: false, planException));
|
||||
var notification = new RuntimeUnattendedUpgradeNotification();
|
||||
|
||||
await sut.HandleAsync(notification, CancellationToken.None);
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
// The failed (but not thrown) package plan must surface as a boot failure, mirroring the core upgrade path,
|
||||
// forwarding the original migration exception rather than swallowing it.
|
||||
runtimeState.Verify(
|
||||
x => x.Configure(RuntimeLevel.BootFailed, RuntimeLevelReason.BootFailedOnException, planException),
|
||||
Times.Once);
|
||||
Assert.That(notification.UnattendedUpgradeResult, Is.EqualTo(RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors));
|
||||
});
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task RunPackageMigrations_WhenMultiplePlansFail_SetsBootFailedWithAllFailures()
|
||||
{
|
||||
var firstException = new InvalidOperationException("first plan exploded");
|
||||
var secondException = new InvalidOperationException("second plan exploded");
|
||||
|
||||
(UnattendedUpgrader sut, Mock<IRuntimeState> runtimeState) = CreateScenario(
|
||||
new PlanSpec("First Failing Package", Successful: false, firstException),
|
||||
new PlanSpec("Second Failing Package", Successful: false, secondException));
|
||||
var notification = new RuntimeUnattendedUpgradeNotification();
|
||||
|
||||
Func<Exception?> capturedError = CaptureBootFailedException(runtimeState);
|
||||
|
||||
await sut.HandleAsync(notification, CancellationToken.None);
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(notification.UnattendedUpgradeResult, Is.EqualTo(RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors));
|
||||
|
||||
// Both failures must be reported together so the operator can address them in one pass.
|
||||
var aggregate = capturedError() as AggregateException;
|
||||
Assert.That(aggregate, Is.Not.Null, "Expected the boot failure to aggregate all failed plans.");
|
||||
Assert.That(aggregate!.InnerExceptions, Does.Contain(firstException));
|
||||
Assert.That(aggregate.InnerExceptions, Does.Contain(secondException));
|
||||
});
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task RunPackageMigrations_WhenPlanFailsWithoutException_SetsBootFailedWithFallbackException()
|
||||
{
|
||||
const string planName = "Failing Package Without Exception";
|
||||
(UnattendedUpgrader sut, Mock<IRuntimeState> runtimeState) =
|
||||
CreateScenario(new PlanSpec(planName, Successful: false, Exception: null));
|
||||
var notification = new RuntimeUnattendedUpgradeNotification();
|
||||
|
||||
Func<Exception?> capturedError = CaptureBootFailedException(runtimeState);
|
||||
|
||||
await sut.HandleAsync(notification, CancellationToken.None);
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(notification.UnattendedUpgradeResult, Is.EqualTo(RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors));
|
||||
|
||||
// A plan can fail without carrying an exception; fall back to a descriptive one naming the plan.
|
||||
Exception? error = capturedError();
|
||||
Assert.That(error, Is.InstanceOf<UnattendedInstallException>());
|
||||
Assert.That(error!.Message, Does.Contain(planName));
|
||||
});
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task RunPackageMigrations_WhenPlanSucceeds_DoesNotSetBootFailed()
|
||||
{
|
||||
(UnattendedUpgrader sut, Mock<IRuntimeState> runtimeState) =
|
||||
CreateScenario(new PlanSpec("Succeeding Package", Successful: true, Exception: null));
|
||||
var notification = new RuntimeUnattendedUpgradeNotification();
|
||||
|
||||
await sut.HandleAsync(notification, CancellationToken.None);
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
runtimeState.Verify(
|
||||
x => x.Configure(RuntimeLevel.BootFailed, It.IsAny<RuntimeLevelReason>(), It.IsAny<Exception?>()),
|
||||
Times.Never);
|
||||
Assert.That(notification.UnattendedUpgradeResult, Is.EqualTo(RuntimeUnattendedUpgradeNotification.UpgradeResult.PackageMigrationComplete));
|
||||
});
|
||||
}
|
||||
|
||||
private static (UnattendedUpgrader Sut, Mock<IRuntimeState> RuntimeState) CreateScenario(params PlanSpec[] specs)
|
||||
{
|
||||
var plans = specs.Select(spec => new TestPackageMigrationPlan(spec.Name)).ToArray();
|
||||
var planCollection = new PackageMigrationPlanCollection(() => plans);
|
||||
Dictionary<string, PlanSpec> specsByName = specs.ToDictionary(spec => spec.Name);
|
||||
|
||||
var migrationExecutor = new Mock<IMigrationPlanExecutor>();
|
||||
migrationExecutor
|
||||
.Setup(x => x.ExecutePlanAsync(It.IsAny<MigrationPlan>(), It.IsAny<string>()))
|
||||
.ReturnsAsync((MigrationPlan executedPlan, string initialState) =>
|
||||
{
|
||||
PlanSpec spec = specsByName[executedPlan.Name];
|
||||
return new ExecutedMigrationPlan
|
||||
{
|
||||
Plan = executedPlan,
|
||||
InitialState = initialState,
|
||||
FinalState = spec.Successful ? "done" : initialState,
|
||||
Successful = spec.Successful,
|
||||
Exception = spec.Exception,
|
||||
CompletedTransitions = Array.Empty<MigrationPlan.Transition>(),
|
||||
};
|
||||
});
|
||||
|
||||
var scopeProvider = new Mock<ICoreScopeProvider>();
|
||||
scopeProvider
|
||||
.Setup(x => x.CreateCoreScope(
|
||||
It.IsAny<IsolationLevel>(),
|
||||
It.IsAny<RepositoryCacheMode>(),
|
||||
It.IsAny<IEventDispatcher?>(),
|
||||
It.IsAny<IScopedNotificationPublisher?>(),
|
||||
It.IsAny<bool?>(),
|
||||
It.IsAny<bool>(),
|
||||
It.IsAny<bool>()))
|
||||
.Returns(Mock.Of<ICoreScope>());
|
||||
|
||||
var packageMigrationRunner = new PackageMigrationRunner(
|
||||
Mock.Of<IProfilingLogger>(),
|
||||
scopeProvider.Object,
|
||||
new PendingPackageMigrations(NullLogger<PendingPackageMigrations>.Instance, planCollection),
|
||||
planCollection,
|
||||
migrationExecutor.Object,
|
||||
Mock.Of<IKeyValueService>(),
|
||||
Mock.Of<IEventAggregator>(),
|
||||
NullLogger<PackageMigrationRunner>.Instance);
|
||||
|
||||
Mock<IRuntimeState> runtimeState = CreateRuntimeState(specs.Select(spec => spec.Name).ToArray());
|
||||
|
||||
var sut = new UnattendedUpgrader(
|
||||
Mock.Of<IProfilingLogger>(),
|
||||
Mock.Of<IUmbracoVersion>(),
|
||||
databaseBuilder: null!, // Unused on the package-migration branch exercised by these tests.
|
||||
runtimeState.Object,
|
||||
packageMigrationRunner,
|
||||
Options.Create(new UnattendedSettings()),
|
||||
CreateDistributedCache(),
|
||||
NullLogger<UnattendedUpgrader>.Instance);
|
||||
|
||||
return (sut, runtimeState);
|
||||
}
|
||||
|
||||
private static Mock<IRuntimeState> CreateRuntimeState(IReadOnlyList<string> pendingPlanNames)
|
||||
{
|
||||
var mock = new Mock<IRuntimeState>();
|
||||
mock.SetupGet(x => x.Level).Returns(RuntimeLevel.Upgrading);
|
||||
mock.SetupGet(x => x.Reason).Returns(RuntimeLevelReason.UpgradePackageMigrations);
|
||||
mock.SetupGet(x => x.StartupState).Returns(new Dictionary<string, object>
|
||||
{
|
||||
[RuntimeState.PendingPackageMigrationsStateKey] = pendingPlanNames,
|
||||
});
|
||||
return mock;
|
||||
}
|
||||
|
||||
private static Func<Exception?> CaptureBootFailedException(Mock<IRuntimeState> runtimeState)
|
||||
{
|
||||
Exception? captured = null;
|
||||
runtimeState
|
||||
.Setup(x => x.Configure(RuntimeLevel.BootFailed, It.IsAny<RuntimeLevelReason>(), It.IsAny<Exception?>()))
|
||||
.Callback<RuntimeLevel, RuntimeLevelReason, Exception?>((_, _, exception) => captured = exception);
|
||||
return () => captured;
|
||||
}
|
||||
|
||||
private static DistributedCache CreateDistributedCache()
|
||||
{
|
||||
static ICacheRefresher Refresher(Guid id)
|
||||
{
|
||||
var mock = new Mock<ICacheRefresher>();
|
||||
mock.SetupGet(x => x.RefresherUniqueId).Returns(id);
|
||||
return mock.Object;
|
||||
}
|
||||
|
||||
var refreshers = new CacheRefresherCollection(() => new[]
|
||||
{
|
||||
Refresher(ContentCacheRefresher.UniqueId),
|
||||
Refresher(MediaCacheRefresher.UniqueId),
|
||||
Refresher(DomainCacheRefresher.UniqueId),
|
||||
});
|
||||
|
||||
return new DistributedCache(Mock.Of<IServerMessenger>(), refreshers);
|
||||
}
|
||||
|
||||
private sealed record PlanSpec(string Name, bool Successful, Exception? Exception);
|
||||
|
||||
private sealed class TestPackageMigrationPlan : PackageMigrationPlan
|
||||
{
|
||||
public TestPackageMigrationPlan(string planName)
|
||||
: base(planName)
|
||||
{
|
||||
}
|
||||
|
||||
protected override void DefinePlan() => To<NoOpMigration>("done");
|
||||
}
|
||||
|
||||
private sealed class NoOpMigration : AsyncMigrationBase
|
||||
{
|
||||
public NoOpMigration(IMigrationContext context)
|
||||
: base(context)
|
||||
{
|
||||
}
|
||||
|
||||
protected override async Task MigrateAsync()
|
||||
{
|
||||
await Task.CompletedTask;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user