diff --git a/.github/workflows/build-test-pack.yml b/.github/workflows/build-test-pack.yml index e457a436..ad9947e7 100644 --- a/.github/workflows/build-test-pack.yml +++ b/.github/workflows/build-test-pack.yml @@ -48,9 +48,11 @@ jobs: - name: Build, test, and pack with Buildvana shell: bash run: | - if [ -z "$BV_VERBOSITY" ]; then BV_VERBOSITY=Normal; fi - if [ "$RUNNER_DEBUG" = "1" ]; then BV_VERBOSITY=Diagnostic; fi - dotnet bv pack --verbosity "$BV_VERBOSITY" + if [ "$RUNNER_DEBUG" = "1" ]; then + dotnet bv pack --verbosity diagnostic + else + dotnet bv pack + fi - name: Upload coverage to Codecov if: ${{ hashFiles('TestResults/*.cobertura.xml') != '' }} uses: codecov/codecov-action@v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 62f1ffad..3d2aca7b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,8 +26,10 @@ on: description: 'Buildvana verbosity' required: true type: choice - default: 'normal' + default: 'minimal' options: + - quiet + - minimal - normal - detailed - diagnostic @@ -81,7 +83,6 @@ jobs: id: build shell: bash run: | - if [ -z "$BV_VERBOSITY" ]; then BV_VERBOSITY=normal; fi if [ "$RUNNER_DEBUG" = "1" ]; then BV_VERBOSITY=diagnostic; fi dotnet bv release --verbosity "$BV_VERBOSITY" --bump "$BV_BUMP" - name: Upload coverage to Codecov diff --git a/CHANGELOG.md b/CHANGELOG.md index 95188ed2..bd418c16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,10 +43,12 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - Before running any command that uses Buildvana SDK (`restore`, `build`, `test`, `pack`, and `release`), `bv` now verifies that the repository pins the SDK (the `Buildvana.Sdk` entry under `msbuild-sdks` in `global.json`) at its own version: `bv`, `Buildvana.Sdk`, and `Buildvana.Runtime` are released in lockstep, and a version mismatch — a half-updated repository, a newer globally-installed tool against an older pin — would otherwise produce silent behavior drift. A missing `global.json`, section, or entry counts as a mismatch. On mismatch, the command fails with a message naming both versions and the ways to align them. Versions are compared by SemVer precedence, ignoring build metadata. The new global option `--skip-sdk-check` skips the check, for scenarios that require a deliberate mismatch (e.g. bisecting an SDK regression in CI). - `bv` now delegates to the repository's pinned version: whenever the tool manifest (`.config/dotnet-tools.json`) pins `bv`, the pinned version is the one that runs, no matter which `bv` is invoked — like the Angular CLI, where the global `ng` always hands over to the project-local install. The invoked `bv` makes sure the pinned version is installed — probing the SDK's tool resolver cache the same way `dotnet tool run` does, and running `dotnet tool restore` only when needed; a failed restore is reported but does not block the attempt — and hands it the entire original command line (`dotnet tool run bv`) with inherited standard streams, forwarding its exit code. The delegated `bv` runs from the home directory, so a relative path inside forwarded arguments resolves against the home directory rather than the invocation directory; and `--version` answers for the pinned `bv` (pass `--skip-delegation` to ask the invoked binary). When the versions differ, an info line on standard error names the version that runs. A delegating `bv` does not judge the command line beyond the minimal split that finds the subcommand and the global options (only a value-bearing global option with no following value, such as a trailing `-v`, is rejected before delegation, with the same message in every version), and does not read the configuration file — both may be valid for the pinned version and not for the invoked one, and judging them is the pinned version's job. The new `update` subcommand is exempt (see below); the new global option `--skip-delegation` runs the exact binary invoked; and the `BV_DELEGATED` environment variable, set on the delegated child, guarantees that a delegated invocation never delegates again. The variable is removed from the environment of every other child process `bv` spawns, so a `bv` reached through a hook or a build makes its own delegation decision. A `bv` invoked outside a repository, or in a repository whose tool manifest does not pin `bv` (the entry is matched case-insensitively, like the dotnet CLI matches it; an entry with an unusable version is reported and treated as no pin), runs in place as before. - A new `bv update` command updates the repository's entire Buildvana surface to the running `bv`'s version in one operation: the `bv` pin in the tool manifest, via `dotnet tool update` (or `dotnet tool install --create-manifest-if-needed` when there is no entry yet), which also downloads the version; the `Buildvana.Sdk` pin in `global.json`, creating the file and/or the `msbuild-sdks` section if needed and preserving the file's formatting otherwise; and the version segment of the configuration file's `$schema` reference, when it points at the canonical `Tenacom/Buildvana//schemas/` URL. Afterwards, the configuration file is loaded with the new version's model, and any problems are reported as warnings for review. `update` is exempt from delegation — it updates the repository to the `bv` actually invoked ("bring this repository to me"), so the usual upgrade flow is `dotnet tool update -g bv` followed by `bv update`, and `dnx bv@ update` targets any specific version — and it refuses to downgrade a repository whose pins are newer than the running `bv`, unless `--force` is passed. A manifest whose `bv` entry pins an invalid version is beyond the dotnet CLI's reach entirely (the CLI cannot parse such a manifest), so `bv update` fails up front with a message naming the entry to fix. -- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity, and the command defaults to minimal verbosity so that by default the report is all there is (pass `--verbosity normal` or higher for diagnostics). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. +- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity and alone on standard output, so it stays pipeable whatever the verbosity (diagnostics go to standard error; pass `--verbosity normal` or higher for more of them). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. ### Changes to existing features +- **BREAKING CHANGE**: `bv` now defaults to `minimal` verbosity, for every command alike; it used to default to `normal`. The build pipeline commands wrap `dotnet restore`/`build`/`test`/`pack`, which default to `minimal` themselves and receive `bv`'s verbosity verbatim, so `bv build` used to produce a markedly noisier MSBuild log than plain `dotnet build`; it now produces a comparable one. Nothing fails and no migration is required: pass `-v normal` for the previous output. Note that `bv`'s own narration — activity start/finish lines and the `info:` lines describing what the tool is doing — is hidden at the new default, while the record of what a command _did_ survives it (see the new `notice:` level below), so `bv release` still logs its complete account of what it changed and published. +- A new message level, rendered as `notice:`, sits between `warning:` and `info:` and is shown from `minimal` verbosity up. It carries the messages that record a fact — something changed, something was decided, something was deliberately skipped — as opposed to the narration of what a command is doing at a given moment, which stays at `info:`. This is also the level at which the version computed for a build is reported, so that line keeps appearing in `dotnet build` output at its default verbosity, as it did before. Buildvana SDK tasks and `bv` now agree on the verbosity at which each level becomes visible: previously a message logged by a task showed up one rung earlier than the same message printed by `bv`. - **BREAKING CHANGE**: The `.buildvana-home` marker file is no longer recognized: home-directory discovery now only looks for a Buildvana configuration file (in the home directory root or in its `.buildvana` subdirectory) and Git markers. `.buildvana-home` predates the configuration file, and a `buildvana.json` containing an empty object (`{}`) does the same job — marking a directory as home without configuring anything — while being what one would naturally reach for. To migrate, replace `.buildvana-home` with a `buildvana.json` file containing `{}`. - **BREAKING CHANGE**: The `JetBrainsAnnotations` module no longer adds any JetBrains annotations package to your project, and the `UseJetBrainsAnnotations` property has been removed. To export ReSharper external annotations, reference an annotations source yourself — the compiled `JetBrains.Annotations` package, `JetBrains.Annotations.Sources`, or your own attributes in the `JetBrains.Annotations` namespace — and set the new boolean `ExportJetBrainsAnnotations` property (default `false`) to `true`. When enabled, Buildvana SDK reads the annotations from source with Roslyn after each build and packs a `{AssemblyName}.ExternalAnnotations.xml` file next to the assembly in `lib/`, one per target framework. The export no longer depends on Mono.Cecil or on a second build pass; and, when the annotation attributes are `[Conditional("JETBRAINS_ANNOTATIONS")]` (as in the JetBrains packages), no JetBrains attribute metadata remains in the compiled assembly, leaving clean IL and AOT output. - **BREAKING CHANGE**: The `JetBrainsAnnotations` module no longer supports Visual Basic projects. `ExportJetBrainsAnnotations` is forced to `false` for any project that is not a C# (`.csproj`) project, because the exporter reads C# source directly. @@ -80,7 +82,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features Cake verbosity values (e.g., `verbose`) are no longer accepted. - `bv` no longer prefixes its console output with a log level and a class-name category (e.g. `info: Buildvana.Tool.Services.DotNetService: ...`). Messages now render as clean, color-coded lines: errors in red and warnings in yellow, each line tagged with a short level label (`error:`/`warning:`/`info:`/`detail:`/`trace:`). In addition, `dotnet`/MSBuild output is now streamed through live (standard output to `bv`'s standard output, standard error to its standard error) instead of being hidden unless the build fails; on failure, the first and last lines of the captured output are still included in the error message. Verbosity behavior is unchanged (`--verbosity quiet|minimal|normal|detailed|diagnostic`), as is the handling of `--color`/`--no-color` (with the [`NO_COLOR` environment variable](https://no-color.org) now honored as well). - **BREAKING CHANGE**: `bv` now writes all of its own narration to standard error, keeping standard output for actual results, per the prevailing CLI convention (git, npm, cargo, etc.). Narration comprises the leveled diagnostic lines (`error:`/`warning:`/`info:`/`detail:`/`trace:`), activity start/finish lines, and the startup logo. Standard output now carries only command deliverables (e.g. `bv version show`'s report) and the standard output of child `dotnet` processes, which is the payload of the build commands. Results thus stay pipeable at any verbosity: `bv version show | some-parser` receives only the report, and `bv build 2>bv.log` separates `bv`'s narration from `dotnet`'s output. Scripts and CI steps that captured diagnostics from `bv`'s standard output must now capture standard error instead (e.g. via `2>&1`). Color auto-detection consequently probes standard error: a redirected standard error disables color, while a redirected standard output no longer does. -- **BREAKING CHANGE**: `bv restore`, `bv build`, `bv test`, and `bv pack` forward extra command-line arguments to the underlying `dotnet` invocation(s) only after a `--` separator: everything after the first `--` is passed through verbatim, in the order given, and `bv` no longer parses or validates it. A non-global, option-looking token _before_ `--` is now an error that points you at the separator. Malformed or unknown forwarded arguments produce an error from `dotnet` (or, for `bv test`, from the Microsoft.Testing.Platform test application) rather than from `bv`. Previously only `-p:`/`/p:` MSBuild properties were forwarded. `bv` also always forwards `--nologo` and its resolved `--verbosity` (default `normal`) to those invocations. +- **BREAKING CHANGE**: `bv restore`, `bv build`, `bv test`, and `bv pack` forward extra command-line arguments to the underlying `dotnet` invocation(s) only after a `--` separator: everything after the first `--` is passed through verbatim, in the order given, and `bv` no longer parses or validates it. A non-global, option-looking token _before_ `--` is now an error that points you at the separator. Malformed or unknown forwarded arguments produce an error from `dotnet` (or, for `bv test`, from the Microsoft.Testing.Platform test application) rather than from `bv`. Previously only `-p:`/`/p:` MSBuild properties were forwarded. `bv` also always forwards `--nologo` and its resolved `--verbosity` (default `minimal`) to those invocations. - `bv build -- -m:8 -v:minimal` forwards `-m:8 -v:minimal` to `dotnet build`. - `bv test -- --report-trx` reaches the test application. - **BREAKING CHANGE**: `bv release` rejects a `--` separator (and anything after it): unlike the pipeline commands, it has no underlying `dotnet` pass-through to forward arguments to. @@ -104,6 +106,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - `bv release` no longer tags and publishes a version one patch above the one its artifacts were built with. The "Prepare release" commit bumps the Git height, hence the version, but it was only created when an earlier step had a file to commit; a release with nothing to commit before the build (typically a prerelease with no version-spec change and `release.changelogUpdates` set to `stable` or `none`) therefore built and pushed its packages at the pre-commit version, then created the commit, and tagged and released the version above. The release commit is now always created before the build. - `bv release` no longer tags and publishes a version one patch _below_ the one its artifacts were built with when the release applies a version-spec change (`--bump minor` or `--bump major`, or a minor bump forced by an additive public API change). The `VERSION` file reached the "Prepare release" commit only after the version had been computed, so at computation time the new version line had no commit carrying it, and its Git height came out as 0 — hence versions such as `2.4.0-preview`, which the height calculation can never legitimately produce, since it counts from 1 at the bump commit and reserves 0 for a version line with no committed history. The artifacts, built after the commit, correctly carried `2.4.1-preview`, while the tag, the release, and the hook args said `2.4.0-preview`; produced-package discovery, which matches packages by version, therefore found none, and the self-reference (dogfood) updates were silently skipped. Files are now staged before the release commit is created, and the version is refreshed after every change to the commit's contents, so what is tagged and published is always what a build of the tagged commit produces. - `bv release` now refuses to publish a version whose Git height is 0, i.e. one whose version line is carried by no commit in the branch's history — typically because `VERSION` has never been committed. Such a version cannot be reproduced: a build of the tagged commit would count the height from 1 as soon as the file reached it, and produce a different version. Building in that state remains perfectly legitimate — it is what a working tree looks like between `bv version advance` and the commit of its result — so only releasing is refused, with a message naming the file to commit. +- `bv release` no longer fails _after_ publishing a release when `GITHUB_OUTPUT` is not set. The variable was read where it is used — appending the `version` step output, which happens once the GitHub release has been published — so an unset variable failed a release that had otherwise succeeded, and the failure rolled it back, deleting the release and the tag it had just created. It is now required up front, before the provisional draft release is created and before any change to the repository, where nothing has to be undone. The message is the usual one for a missing variable (`Required environment variable GITHUB_OUTPUT is not set or empty.`), replacing `Cannot set Actions step output: GITHUB_OUTPUT not set.` - URLs that `bv release` builds from the repository URL are no longer missing the separator before their first path segment: release links (`.../Buildvanareleases/tag/1.1.10`) and file links (`.../Buildvanablob/main/CHANGELOG.md`) now come out as `.../Buildvana/releases/tag/1.1.10` and `.../Buildvana/blob/main/CHANGELOG.md`. This affected the version section titles written into the changelog and the "human-curated changelog" link at the top of every generated release description; the titles already written for 1.0.220, 1.1.4, and 1.1.10 have been corrected in place. - `bv clean` no longer silently ignores unknown options: `bv clean --bogus` now fails with `Unknown option '--bogus' for command 'clean'`. Every `bv` command now rejects options it does not recognize, and does so before anything else runs: previously, commands that parse their own options (e.g. `bv release`) reported an unknown option only after the SDK version check, so a mismatched SDK pin could mask the typo. - A denied or failed file or directory access during a `bv` command (e.g. a locked or read-only `CHANGELOG.md`, `Directory.Packages.props`, or public API file, or a `bin` directory locked by Visual Studio during `bv clean`) no longer surfaces as an unhandled-exception stack trace pointing at `bv` internals. File and directory accesses now report failure as a single clean error line naming the operation, the path, and the operating-system reason (`Could not read from : `), and `bv` exits with its regular failure exit code. This covers failures that happen part-way through reading a file, not just failures to open it: `bv release` reads the whole of `CHANGELOG.md` before rewriting it, so a file yanked mid-read (say, by a cloud-sync provider) is reported the same clean way. diff --git a/docs/EnvironmentVariables.md b/docs/EnvironmentVariables.md index 874e6dfe..7b0d99df 100644 --- a/docs/EnvironmentVariables.md +++ b/docs/EnvironmentVariables.md @@ -36,7 +36,7 @@ Set to `true` by GitHub Actions on every step. `bv` reads it to recognize that i ### `GITHUB_OUTPUT` -Set by GitHub Actions to the path of the file that collects a step's outputs. `bv release` appends to that file to publish the released version as the `version` step output, so that later steps of the same job can refer to it; the release fails if the variable is unset. `bv` never sets this variable itself. +Set by GitHub Actions to the path of the file that collects a step's outputs. `bv release` appends to that file to publish the released version as the `version` step output, so that later steps of the same job can refer to it; the release fails if the variable is unset, and fails up front, before creating anything, rather than at the moment the output is written. `bv` never sets this variable itself. ### `GITLAB_CI` diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs index df74212e..aa083c65 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs @@ -8,20 +8,41 @@ namespace Buildvana.Core.ConsoleOutput; /// message and, together with the reporter's , decides whether it is shown. /// /// -/// Members are ordered from highest to lowest severity, mapping one-to-one onto the -/// thresholds: ↔, ↔, -/// ↔, ↔, -/// ↔. +/// Members are ordered from highest to lowest severity. There are more levels than there are +/// thresholds, so the two do not map one-to-one: the verbosity from which each level +/// becomes visible is stated by , which is the single +/// authority on the matter and the one every implementation agrees on. /// public enum MessageLevel { /// An error: something went wrong. Shown at every verbosity. Error, - /// A warning: something looks off but is not fatal. + /// + /// A warning: something looks off but is not fatal. Shown at and above. + /// Warning, - /// An informational milestone. Shown at and above. + /// + /// A record of a fact: something changed, something was decided, something was deliberately skipped. + /// Shown at and above. + /// + /// + /// Use this level for what the reader would want to know afterwards — the version spec changed, N files + /// were rewritten, a step was skipped and why — and for narration of what the tool is doing + /// right now. The two are not a loudness ranking: this is a quieter , not a louder + /// , and it should feel like it costs something. A message at this level survives the default + /// verbosity, so promoting narration to it makes the default as noisy as and + /// leaves the ladder with no rung meaning what this one means. + /// + Notice, + + /// + /// Narration of what the tool is doing right now. Shown at and above. + /// + /// + /// See for the criterion that separates the two levels. + /// Info, /// A detail useful when following along closely. Shown at and above. diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs new file mode 100644 index 00000000..4826db62 --- /dev/null +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs @@ -0,0 +1,50 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System; + +namespace Buildvana.Core.ConsoleOutput; + +/// +/// Provides extension methods for values. +/// +#pragma warning disable CA1034 // Nested types should not be visible — false positive on C# 14 extension blocks; fixed in .NET 11, backport to .NET 10 requested in https://github.com/dotnet/sdk/issues/53984 +#pragma warning disable CA1708 // Identifiers should differ by more than case — false positive on classes with C# 14 extension blocks; fixed in .NET 11, https://github.com/dotnet/sdk/issues/51716 +public static class MessageLevelExtensions +{ + extension(MessageLevel @this) + { + /// + /// Gets the least verbose at which a message of this level is shown. + /// + /// The minimum that enables this level. + /// This level is not a known . + /// + /// This method is the single authority on when a level becomes visible. Every + /// implementation answers the question through it — directly, or by mapping the + /// answer onto the visibility rules of whatever it renders through — so that a given level becomes + /// visible at the same point no matter which reporter is in play. + /// The mapping cannot be a comparison of the two enums' underlying values: there are more levels + /// than there are thresholds, so no ordering of the members makes such a comparison give the right + /// answer for all of them. + /// + public Verbosity MinimumVerbosity() + { + return @this switch + { + MessageLevel.Error => Verbosity.Quiet, + MessageLevel.Warning or MessageLevel.Notice => Verbosity.Minimal, + MessageLevel.Info => Verbosity.Normal, + MessageLevel.Detail => Verbosity.Detailed, + MessageLevel.Trace => Verbosity.Diagnostic, + _ => ThrowUnknownLevel(@this), + }; + + // The exception names the offending value "level", as every caller of this method does. The name + // cannot come from the receiver: nameof(@this) yields "this", which names the parameter the + // compiler emits rather than anything a caller can see. + static Verbosity ThrowUnknownLevel(MessageLevel level) + => throw new ArgumentOutOfRangeException(nameof(level), level, "Unknown message level."); + } + } +} diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs index 6ca74fc0..f2ccf268 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs @@ -29,6 +29,10 @@ public static class ReporterExtensions /// The message text. public void Warning(string message) => @this.Report(MessageLevel.Warning, message); + /// Reports a message. + /// The message text. + public void Notice(string message) => @this.Report(MessageLevel.Notice, message); + /// Reports an message. /// The message text. public void Info(string message) => @this.Report(MessageLevel.Info, message); @@ -53,6 +57,12 @@ public void Error(CompositeFormat format, params ReadOnlySpan args) public void Warning(CompositeFormat format, params ReadOnlySpan args) => @this.Report(MessageLevel.Warning, format, args); + /// Formats and reports a message. + /// The composite format string. + /// The arguments to format. + public void Notice(CompositeFormat format, params ReadOnlySpan args) + => @this.Report(MessageLevel.Notice, format, args); + /// Formats and reports an message. /// The composite format string. /// The arguments to format. @@ -95,7 +105,8 @@ public void Report(MessageLevel level, CompositeFormat format, params ReadOnlySp /// /// The level to test. /// if the level is enabled; otherwise, . - public bool IsEnabled(MessageLevel level) => (int)level <= (int)@this.Verbosity; + /// is not a known . + public bool IsEnabled(MessageLevel level) => @this.IsVerbosityAtLeast(level.MinimumVerbosity()); /// /// Determines whether the reporter's is at least the given diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs index d468d597..b2e2b0e3 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs @@ -5,22 +5,23 @@ namespace Buildvana.Core.ConsoleOutput; /// /// Controls how much of a reporter's output reaches the user. Each level enables all the -/// s enabled by the levels below it (see for the mapping). +/// s enabled by the levels below it. /// /// -/// The members mirror bv's --verbosity command-line vocabulary and are ordered from least to most -/// verbose, so a message at a given is shown when -/// (int)level <= (int)verbosity. +/// The members mirror bv's --verbosity command-line vocabulary and are ordered from least to +/// most verbose. Which levels each one enables is stated by +/// : a message is shown when its level's minimum verbosity +/// is at most the one in effect. /// public enum Verbosity { /// Only errors are shown. Quiet, - /// Errors and warnings are shown. + /// Errors, warnings, and notices are shown. This is bv's default, as it is the .NET CLI's. Minimal, - /// Errors, warnings, and informational messages are shown. This is the default. + /// Everything shows, plus informational messages. Normal, /// Everything shows, plus detail messages. diff --git a/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs b/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs index f3d92188..14d40ce6 100644 --- a/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs +++ b/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs @@ -133,6 +133,7 @@ public void ChildError(string line, Verbosity? minimumVerbosity) { MessageLevel.Error => (ConsoleColor.Red, "error"), MessageLevel.Warning => (ConsoleColor.Yellow, "warning"), + MessageLevel.Notice => (null, "notice"), MessageLevel.Info => (null, "info"), MessageLevel.Detail => (null, "detail"), MessageLevel.Trace => (null, "trace"), diff --git a/src/Buildvana.Core.Versioning/VersioningService.cs b/src/Buildvana.Core.Versioning/VersioningService.cs index 5e7ab55b..18c97978 100644 --- a/src/Buildvana.Core.Versioning/VersioningService.cs +++ b/src/Buildvana.Core.Versioning/VersioningService.cs @@ -59,7 +59,7 @@ public VersioningService( FileVersion = FormattableString.Invariant($"{SimpleVersion}.0"); InformationalVersion = ComputeInformationalVersion(SemVer, Spec.Prerelease, IsPublicRelease, CommitId); var publicity = IsPublicRelease ? "public release" : "not a public release"; - reporter.Info(FormattableString.Invariant($"Version {SemVer} (height {Height}, {publicity})")); + reporter.Notice(FormattableString.Invariant($"Version {SemVer} (height {Height}, {publicity})")); } /// diff --git a/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs b/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs index be95cf61..5231f746 100644 --- a/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs +++ b/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs @@ -16,18 +16,36 @@ namespace Buildvana.Sdk; /// human-facing output through the build's own loggers. /// /// -/// Message levels map to MSBuild severities as follows: and -/// become build errors and warnings; , -/// , and become messages of -/// , , and -/// importance respectively. +/// Message levels map to MSBuild severities so that each level becomes visible at the verbosity named by +/// , the same one at which bv's console shows it: +/// and become build errors and warnings; +/// becomes a message of importance, which +/// MSBuild shows from minimal verbosity up; becomes +/// , shown from normal verbosity up; and both +/// and become +/// , shown from detailed verbosity up. +/// The last two share a rung because MSBuild's ladder has three and ends there. A task cannot do better: +/// exposes no verbosity to read, and its +/// is a union query across every registered logger, each +/// with a verbosity of its own — so detailed and diagnostic are indistinguishable from in here, and +/// surfaces one rung earlier than it would on the console. /// Messages are always forwarded, regardless of : visibility is governed by /// MSBuild's own verbosity and importance filtering, exactly like any other message logged by a task. /// is derived from when available /// (falling back to fully permissive otherwise), so that callers checking it — such as the formatting /// helpers in — skip work whose output MSBuild would discard anyway. +/// That derivation over-claims at both ends of the ladder, deliberately, because its only consumers are +/// short-circuit checks and the cost of guessing low is a dropped message. When low-importance messages are +/// logged it reports rather than : the honest +/// answer would make Report(Trace, format, args) short-circuit and drop every formatted +/// message even under -v:diag. When nothing at all is logged it floors at +/// rather than : the honest answer would make +/// Warning(format, args) short-circuit and drop warnings that MSBuild's own quiet verbosity still +/// prints. /// Activity start and outcome lines are logged at importance, -/// so they are hidden at MSBuild's default (minimal) verbosity and visible from normal verbosity up. +/// so they are hidden at MSBuild's default (minimal) verbosity and visible from normal verbosity up. That is the +/// importance maps to, and the console side gates the same lines on +/// too, so the two agree on when an activity is narrated. /// Child-process output and error lines are both forwarded as low-importance messages: MSBuild has no /// neutral standard-error channel, and logging a build error or warning would misrepresent — and, given how /// task success is determined, potentially fail the build over — stderr lines that many tools use for @@ -48,9 +66,10 @@ public TaskLoggingHelperReporter(TaskLoggingHelper log, IBuildEngine engine) _engineServices = (engine as IBuildEngine10)?.EngineServices; } + // The two remaining rungs of MSBuild's ladder, minimal and quiet, both answer Minimal here: see the + // over-claim paragraph in this class's remarks for why quiet does not answer Quiet. public Verbosity Verbosity => LogsMessagesOfImportance(MessageImportance.Low) ? Verbosity.Diagnostic - : LogsMessagesOfImportance(MessageImportance.Normal) ? Verbosity.Detailed - : LogsMessagesOfImportance(MessageImportance.High) ? Verbosity.Normal + : LogsMessagesOfImportance(MessageImportance.Normal) ? Verbosity.Normal : Verbosity.Minimal; public void Report(MessageLevel level, string message) @@ -66,12 +85,13 @@ public void Report(MessageLevel level, string message) case MessageLevel.Warning: _log.LogWarning("{0}", message); break; - case MessageLevel.Info: + case MessageLevel.Notice: _log.LogMessage(MessageImportance.High, "{0}", message); break; - case MessageLevel.Detail: + case MessageLevel.Info: _log.LogMessage(MessageImportance.Normal, "{0}", message); break; + case MessageLevel.Detail: case MessageLevel.Trace: _log.LogMessage(MessageImportance.Low, "{0}", message); break; diff --git a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs index 180b6591..a2a41954 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs @@ -3,20 +3,18 @@ using System; using System.Collections.Generic; -using Buildvana.Core.ConsoleOutput; namespace Buildvana.Tool.Infrastructure.Execution; /// /// A discovered bv command: the paths it is registered under, the class that implements it, whether -/// it forwards all of its arguments verbatim, its settings type (if any), and its default verbosity. Produced by +/// it forwards all of its arguments verbatim, and its settings type (if any). Produced by /// from . /// /// The paths the command is invoked under, each as a list of segments. The first path is canonical. /// The class implementing the command. /// Whether the command forwards all of its arguments verbatim. /// The command's *Settings type, or if it has none. -/// The verbosity in effect when --verbosity is not given. /// Whether the command uses the repository's pinned Buildvana SDK and must therefore /// pass the SDK version check before running. internal sealed record CommandRegistration( @@ -24,7 +22,6 @@ internal sealed record CommandRegistration( Type CommandType, bool ConsumesAllArguments, Type? SettingsType, - Verbosity DefaultVerbosity = Verbosity.Normal, bool UsesSdk = false) { /// diff --git a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs index ebf8faa8..3fe180ad 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs @@ -190,7 +190,6 @@ private static IReadOnlyList Discover() type, attribute.ConsumesAllArguments, attribute.SettingsType, - attribute.DefaultVerbosity, attribute.UsesSdk)); } } diff --git a/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs b/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs index 5f530b60..0c1b8101 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; -using Buildvana.Core.ConsoleOutput; namespace Buildvana.Tool.Infrastructure.Execution; @@ -41,11 +40,6 @@ internal sealed class ImplementsCommandAttribute : Attribute /// -decorated properties the help renderer /// enumerates; for commands with no options or arguments of their own. /// - /// - /// The verbosity in effect when --verbosity is not given. Commands whose deliverable is their console - /// output (query commands) use so that only their result, warnings, and - /// errors are shown by default. - /// /// /// if the command uses the repository's pinned Buildvana SDK — by running MSBuild /// targets on solution projects, or anything else whose outcome depends on the global.json pin — @@ -57,7 +51,6 @@ public ImplementsCommandAttribute( string aliases, bool consumesAllArguments = false, Type? settingsType = null, - Verbosity defaultVerbosity = Verbosity.Normal, bool usesSdk = false) { ArgumentException.ThrowIfNullOrWhiteSpace(aliases); @@ -77,7 +70,6 @@ public ImplementsCommandAttribute( AliasPaths = paths; ConsumesAllArguments = consumesAllArguments; SettingsType = settingsType; - DefaultVerbosity = defaultVerbosity; UsesSdk = usesSdk; } @@ -101,11 +93,6 @@ public ImplementsCommandAttribute( /// public Type? SettingsType { get; } - /// - /// Gets the verbosity in effect when --verbosity is not given. - /// - public Verbosity DefaultVerbosity { get; } - /// /// Gets a value indicating whether the command uses the repository's pinned Buildvana SDK and must /// therefore pass the SDK version check before running. diff --git a/src/Buildvana.Tool/Program.cs b/src/Buildvana.Tool/Program.cs index a203ae8d..b645a90f 100644 --- a/src/Buildvana.Tool/Program.cs +++ b/src/Buildvana.Tool/Program.cs @@ -31,6 +31,10 @@ internal static class Program // 128 + SIGINT (2): the POSIX convention for a process terminated by Ctrl-C. private const int CancelledExitCode = 130; + // The verbosity in effect when --verbosity is not given, for every command alike. It matches the default of + // `dotnet restore`/`build`/`test`/`pack`, which the build pipeline commands wrap and forward it to. + private const Verbosity DefaultVerbosity = Verbosity.Minimal; + public static async Task Main(string[] args) { // Before anything else, including Spectre's console: replacing the console's encoding resets Console.Out @@ -106,8 +110,10 @@ public static async Task Main(string[] args) CommandArgumentValidator.Validate(command, parsed, positionals); // Parse --verbosity eagerly so an invalid value surfaces in the outer catch. - // When absent, the command's own default applies (query commands default to Minimal). - var verbosity = globals.Verbosity is null ? command.DefaultVerbosity : VerbosityParser.Parse(globals.Verbosity); + // The default is the .NET CLI's, so that a bv command's output is comparable to that of the + // dotnet command underneath it. It is uniform across commands: verbosity is process-wide, and a + // command that runs the build pipeline would restore the noisier log through the back door. + var verbosity = globals.Verbosity is null ? DefaultVerbosity : VerbosityParser.Parse(globals.Verbosity); // --color / --no-color win over auto-detection; neither (or both) leaves the reporter to auto-detect. bool? colorOverride = (globals.Color, globals.NoColor) switch @@ -191,7 +197,7 @@ void OnCancel(object? sender, ConsoleCancelEventArgs e) return ex.ExitCode; } - static IReporter CreateDefaultReporter() => new ConsoleReporter(Verbosity.Normal, colorOverride: null); + static IReporter CreateDefaultReporter() => new ConsoleReporter(DefaultVerbosity, colorOverride: null); } private static Task TryDelegateAsync(string[] args, ParsedCommandLine parsed, GlobalSettings globals) diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index e8e9de34..b31beb5e 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -159,7 +159,7 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati if (!hasTestProjects) { - _reporter.Info("No test projects found, skipping tests."); + _reporter.Notice("No test projects found, skipping tests."); return; } @@ -268,7 +268,7 @@ await RunDotNetAsync( cancellationToken: cancellationToken).ConfigureAwait(false); } - _reporter.Info($"Pushed {packages.Length} packages to {target.Source}."); + _reporter.Notice($"Pushed {packages.Length} packages to {target.Source}."); } /// diff --git a/src/Buildvana.Tool/Services/Git/GitService.cs b/src/Buildvana.Tool/Services/Git/GitService.cs index 41f503f1..af4c2e15 100644 --- a/src/Buildvana.Tool/Services/Git/GitService.cs +++ b/src/Buildvana.Tool/Services/Git/GitService.cs @@ -254,6 +254,8 @@ public void Commit(string message, bool amend = false, bool allowEmpty = false) /// This method's purpose is to undo a commit that was just generated by code and is not a merge commit. /// If the current HEAD has multiple parents, the behavior of this method is undefined. /// If the repository has no commits, or the current HEAD has no parents, this method will fail. + /// Undoing a commit is not an outcome in itself: callers undo a whole set of commits at once, and only + /// the caller knows what that set was, so the record of the undoing belongs to it. This method only narrates. /// public void UndoLastCommit() { @@ -290,11 +292,13 @@ public void Push(bool force = false) _reporter.Info($"Force pushing changes to '{remote}'..."); var pushRefSpec = string.Format(CultureInfo.InvariantCulture, "+{0}:{0}", _repository.Head.CanonicalName); _repository.Network.Push(_repository.Network.Remotes[remote], pushRefSpec, pushOptions); + _reporter.Notice($"Force-pushed '{head.FriendlyName}' to '{remote}'."); } else { _reporter.Info($"Pushing changes to '{remote}'..."); _repository.Network.Push(head, pushOptions); + _reporter.Notice($"Pushed '{head.FriendlyName}' to '{remote}'."); } } diff --git a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs index c179eea6..4aa32094 100644 --- a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs +++ b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs @@ -103,15 +103,16 @@ private async Task RunHookAsync(string context, string @event, object args } var json = JsonSerializer.Serialize(args, args.GetType(), BuildvanaJsonContext.Default); - _reporter.Detail($"Hook {hookName}: args: {json}"); + _reporter.Trace($"Hook {hookName}: args: {json}"); var argsPath = _home.GetFullPath(WellKnownPaths.GetHookArgsFile(context, @event)); _ = UserDirectory.CreateDirectory(Path.GetDirectoryName(argsPath)!); await UserFile.WriteAllTextAsync(argsPath, json, cancellationToken).ConfigureAwait(false); - _reporter.Info($"Hook {hookName}: running {relativePath}..."); + _reporter.Info($"Running hook {hookName}..."); _ = await _appRunner.RunFileBasedAppAsync( path, workingDirectory: _home.HomeDirectory, cancellationToken: cancellationToken).ConfigureAwait(false); + _reporter.Notice($"Hook {hookName} ran."); return true; } } diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index ac5225e2..086adcd0 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -36,7 +36,9 @@ private GitHubServerAdapter(IServiceProvider services) _reporter = services.GetRequiredService(); _version = services.GetRequiredService(); _git = services.GetRequiredService(); - BuildFailedException.ThrowIfNot(GitUrlInfo.TryCreate(_git.OriginUrl, out var originInfo), $"Couldn't get information from origin URL '{_git.OriginUrl}'."); + BuildFailedException.ThrowIfNot( + GitUrlInfo.TryCreate(_git.OriginUrl, out var originInfo), + $"Couldn't get information from origin URL '{_git.OriginUrl}'."); BuildFailedException.ThrowIfNot(originInfo.PathSegments.Count == 2, $"'{originInfo.Url}' is not a valid GitHub repository URL."); HostName = originInfo.Host; RepositoryOwner = originInfo.PathSegments[0]; @@ -68,7 +70,9 @@ private GitHubServerAdapter(IServiceProvider services) public override bool IsCloudBuild => true; /// - public override GitIdentity? CIBotIdentity { get; } = new("github-actions[bot]", "41898282+github-actions[bot]@users.noreply.github.com"); + public override GitIdentity? CIBotIdentity { get; } = new( + "github-actions[bot]", + "41898282+github-actions[bot]@users.noreply.github.com"); /// public override string PushUsername => "x-access-token"; @@ -94,14 +98,17 @@ private GitHubServerAdapter(IServiceProvider services) /// /// Sets a GitHub Actions step output. /// + /// The path of the file that collects the step's outputs, i.e. the value of + /// the GITHUB_OUTPUT environment variable. /// The output name. /// The output value. - public static void SetActionsStepOutput(string name, string value) - { - var outputFile = Environment.GetEnvironmentVariable("GITHUB_OUTPUT"); - BuildFailedException.ThrowIf(string.IsNullOrEmpty(outputFile), "Cannot set Actions step output: GITHUB_OUTPUT not set."); - UserFile.AppendAllLines(outputFile, [$"{name}={value}"], Encoding.UTF8); - } + /// + /// The caller provides the path, instead of this method reading the environment variable itself, + /// so that a missing variable is discovered while there is still nothing to undo. See + /// . + /// + public static void SetActionsStepOutput(string outputFile, string name, string value) + => UserFile.AppendAllLines(outputFile, [$"{name}={value}"], Encoding.UTF8); /// public override async Task IsPrivateRepositoryAsync() @@ -160,7 +167,10 @@ public async Task PublishReleaseAsync(Release release, string targetCommitish) TargetCommitish = targetCommitish, }; - var generateNotesResponse = await client.Repository.Release.GenerateReleaseNotes(RepositoryOwner, RepositoryName, releaseNotesRequest).ConfigureAwait(false); + var generateNotesResponse = await client.Repository.Release.GenerateReleaseNotes( + RepositoryOwner, + RepositoryName, + releaseNotesRequest).ConfigureAwait(false); var body = $"We also have a [human-curated changelog]({GetFileUrl("CHANGELOG.md", _git.CurrentBranch)}).\n\n---\n\n" + generateNotesResponse.Body; @@ -182,6 +192,12 @@ public async Task PublishReleaseAsync(Release release, string targetCommitish) /// An object representing the release. /// The tag name, or to not delete a tag. /// A representing the ongoing operation. + /// + /// A deleted release is named after , never after + /// : the draft is created before the release commit exists, and that commit + /// always moves the version, so the tag name the draft carries is never the one the release is published + /// under. A draft, having no tag of its own, is reported as the draft it is. + /// public async Task DeleteReleaseAsync(Release release, string? tagName) { Guard.IsNotNull(release); @@ -190,9 +206,12 @@ public async Task DeleteReleaseAsync(Release release, string? tagName) await client.Repository.Release.Delete(RepositoryOwner, RepositoryName, release.Id).ConfigureAwait(false); if (string.IsNullOrEmpty(tagName)) { + _reporter.Notice("Deleted the provisional draft release."); return; } + _reporter.Notice($"Deleted release {tagName}."); + var reference = "refs/tags/" + tagName; _reporter.Info($"Looking for reference '{reference}' in GitHub repository..."); try @@ -207,6 +226,7 @@ public async Task DeleteReleaseAsync(Release release, string? tagName) _reporter.Info($"Deleting reference '{reference}' in GitHub repository..."); await client.Git.Reference.Delete(RepositoryOwner, RepositoryName, reference).ConfigureAwait(false); + _reporter.Notice($"Deleted tag {tagName}."); } /// diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs index 685fd7cb..50161d6c 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Services.Versioning; +using Buildvana.Tool.Utilities; using CommunityToolkit.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Octokit; @@ -23,10 +24,15 @@ internal sealed class GitHubServerRelease : ServerRelease private readonly IReporter _reporter; private readonly VersionService _version; private readonly Release _gitHubRelease; + private readonly string _actionsStepOutputFile; private bool _gitHubReleaseDeleted; - private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider services, Release gitHubRelease) + private GitHubServerRelease( + GitHubServerAdapter server, + IServiceProvider services, + Release gitHubRelease, + string actionsStepOutputFile) : base(services) { Guard.IsNotNull(server); @@ -37,6 +43,7 @@ private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider service _reporter = services.GetRequiredService(); _version = services.GetRequiredService(); _gitHubRelease = gitHubRelease; + _actionsStepOutputFile = actionsStepOutputFile; OnRollback(async () => { @@ -48,14 +55,22 @@ private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider service }); } - public static async Task CreateAsync(GitHubServerAdapter server, IServiceProvider services, Func> createGitHubReleaseAsync) + public static async Task CreateAsync( + GitHubServerAdapter server, + IServiceProvider services, + Func> createGitHubReleaseAsync) { Guard.IsNotNull(server); Guard.IsNotNull(services); Guard.IsNotNull(createGitHubReleaseAsync); + // GITHUB_OUTPUT is read here, before the draft release exists, and remembered for the whole + // life of the release. Reading it where it is used - after publication, to write the step + // output - would let a variable that was never set fail a release that had otherwise + // succeeded, and roll back everything it had just done. + var actionsStepOutputFile = EnvVarHelper.Require("GITHUB_OUTPUT"); var gitHubRelease = await createGitHubReleaseAsync().ConfigureAwait(false); - return new(server, services, gitHubRelease); + return new(server, services, gitHubRelease, actionsStepOutputFile); } protected override async Task DoPublishAsync(IReadOnlyList assets) @@ -92,7 +107,7 @@ protected override async Task UndoPublishAsync() protected override Task OnPublishedAsync() { - GitHubServerAdapter.SetActionsStepOutput("version", _version.CurrentStr); + GitHubServerAdapter.SetActionsStepOutput(_actionsStepOutputFile, "version", _version.CurrentStr); return Task.CompletedTask; } } diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index 1f2d1190..f80da846 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Threading.Tasks; using Buildvana.Core.ConsoleOutput; @@ -76,7 +77,8 @@ public void EnsureReleaseCommit() if (_updatesPushed) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot create the release commit when updates have already been pushed."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot create the release commit when updates have already been pushed."); } if (_repositoryUpdated) @@ -99,6 +101,15 @@ public void EnsureReleaseCommit() _git.UndoLastCommit(); } + // The whole set of commits is one undoing, recorded once: a notice per commit would read + // as a repeated line rather than as the several commits it actually walks back. + _reporter.Notice(_postReleaseCommits switch + { + 0 => "Undid the release commit.", + 1 => "Undid the release commit and 1 post-release commit.", + var count => string.Create(CultureInfo.InvariantCulture, $"Undid the release commit and {count} post-release commits."), + }); + // If updates have already been pushed... if (_updatesPushed) { @@ -121,7 +132,8 @@ public void UpdateRepository(params string[] files) if (_postReleaseCommits > 0) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot update the release commit after a post-release commit has been added."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot update the release commit after a post-release commit has been added."); } // Staging comes first, so that the files are already in the index when the release commit is @@ -164,12 +176,14 @@ public void AddPostReleaseCommit(string message, params string[] files) if (_updatesPushed) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot add a post-release commit when updates have already been pushed."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot add a post-release commit when updates have already been pushed."); } if (!_repositoryUpdated) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot add a post-release commit before the release commit has been created."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot add a post-release commit before the release commit has been created."); } _git.Stage(files); @@ -188,7 +202,7 @@ public void PushUpdates() if (!_repositoryUpdated) { - _reporter.Info("Repository unchanged, no commit to push."); + _reporter.Notice("Repository unchanged, no commit to push."); return; } @@ -227,6 +241,16 @@ public async Task PublishAsync() await OnPublishedAsync().ConfigureAwait(false); _published = true; _rollbackActions.Clear(); + + // The record of the publication comes last, once nothing can undo it any more: OnPublishedAsync + // is the final step that can still fail, and its failure rolls the whole release back - deleting + // the release and the tag that a notice printed any earlier would already have claimed. + _reporter.Notice(_assets.Count switch + { + 0 => $"Published release {_version.CurrentStr} with no assets.", + 1 => $"Published release {_version.CurrentStr} with 1 asset.", + var count => string.Create(CultureInfo.InvariantCulture, $"Published release {_version.CurrentStr} with {count} assets."), + }); } public async ValueTask DisposeAsync() diff --git a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs index 1b48f73e..c2873cc5 100644 --- a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs +++ b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs @@ -22,7 +22,7 @@ namespace Buildvana.Tool.Subcommands; /// The constructor parameter order is also the order in which these options appear in bv's help. internal sealed record GlobalSettings( [property: BvOption("-v|--verbosity ")] - [property: Description("Logging verbosity. One of: quiet, minimal, normal, detailed, diagnostic. Defaults to normal.")] + [property: Description("Logging verbosity. One of: quiet, minimal, normal, detailed, diagnostic. Defaults to minimal.")] string? Verbosity, [property: BvOption("--color")] [property: Description("Force ANSI color output even when not connected to a TTY.")] diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index d1707c2d..8bb9a4ae 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -31,7 +31,10 @@ namespace Buildvana.Tool.Subcommands; [ImplementsCommand("release", settingsType: typeof(ReleaseSettings), usesSdk: true)] [Description("Publish a new public release (CI only).")] -internal sealed class ReleaseCommand(IServiceProvider services, ReleaseSettings settings, BuildPipeline pipeline) : IBvCommand +internal sealed class ReleaseCommand( + IServiceProvider services, + ReleaseSettings settings, + BuildPipeline pipeline) : IBvCommand { public async Task ExecuteAsync(CancellationToken cancellationToken) { @@ -63,8 +66,10 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) BuildFailedException.ThrowIfNot(version.IsPublicRelease, "Cannot create a release from the current branch."); // Ensure that the CI bot identity is used for commits, if not already set. - git.CommitterIdentity ??= server.CIBotIdentity ?? throw new BuildFailedException("Cannot determine a committer identity for release commits. Configure git config user.name/user.email before running this task."); - reporter.Info($"Using committer identity: {git.CommitterIdentity.Name} <{git.CommitterIdentity.Email}>"); + git.CommitterIdentity ??= server.CIBotIdentity ?? throw new BuildFailedException( + "Cannot determine a committer identity for release commits. " + + "Configure git config user.name/user.email before running this task."); + reporter.Notice($"Using committer identity: {git.CommitterIdentity.Name} <{git.CommitterIdentity.Email}>"); // Set fallback Git credentials if the server adapter can provide them. var pushUsername = server.PushUsername; @@ -76,7 +81,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { - reporter.Warning("No push credentials provided by the server adapter. Push operations may fail if the repository is not already authenticated."); + reporter.Warning( + "No push credentials provided by the server adapter. " + + "Push operations may fail if the repository is not already authenticated."); } // Perform an initial versioning consistency check. @@ -103,36 +110,32 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) var previousVersionSpec = versionFile.Spec; if (versionFile.ApplyChange(versionSpecChange)) { - reporter.Info($"Version spec changed from {previousVersionSpec} to {versionFile.Spec}."); + reporter.Notice($"Version spec changed from {previousVersionSpec} to {versionFile.Spec}."); versionFile.Save(versioningSettings.PrereleaseTag); release.UpdateRepository(versionFile.Path); } else { - reporter.Info("Version spec not changed."); + reporter.Notice("Version spec not changed."); } } // Update public API files only when releasing a stable version if (version.IsPrerelease) { - reporter.Info("Public API update skipped: not needed on prerelease."); + reporter.Notice("Public API update skipped: not needed on prerelease."); } else { var modified = publicApiFiles.TransferAllPublicApisToShipped().ToArray(); - switch (modified.Length) + + // Never one: the transfer yields both files of every pair it modifies, so there is no + // singular case to report. + reporter.Notice(modified.Length switch { - case 0: - reporter.Info("No public API files were modified."); - break; - case 1: - reporter.Info("1 public API file was modified."); - break; - default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"{modified.Length} public API files were modified.")); - break; - } + 0 => "No public API files were modified.", + var count => string.Create(CultureInfo.InvariantCulture, $"{count} public API files were modified."), + }); if (modified.Length > 0) { @@ -147,14 +150,14 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) && (changelogUpdates == ChangelogUpdates.All || !version.IsPrerelease); if (!changelog.Exists) { - reporter.Info($"Changelog update skipped: {ChangelogService.FileName} not found."); + reporter.Notice($"Changelog update skipped: {ChangelogService.FileName} not found."); } else if (!shouldUpdateChangelog) { var reason = changelogUpdates == ChangelogUpdates.None ? "changelog updates are disabled (release.changelogUpdates is 'none')." : "not needed on prerelease."; - reporter.Info($"Changelog update skipped: {reason}"); + reporter.Notice($"Changelog update skipped: {reason}"); } else { @@ -167,12 +170,14 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { + const string failureMessage = "Changelog check failed: the \"Unreleased changes\" section is empty " + + "or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."; + emptyChangelogSubstitute = settings.ResolveEmptyChangelog(); - BuildFailedException.ThrowIf( - emptyChangelogSubstitute is null, - "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."); + BuildFailedException.ThrowIf(emptyChangelogSubstitute is null, failureMessage); - reporter.Info("Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); + reporter.Notice( + "Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); } // Update the changelog and commit the change before building. @@ -209,7 +214,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { - reporter.Info("Changelog section title update skipped: changelog has not been updated."); + reporter.Notice("Changelog section title update skipped: changelog has not been updated."); } // Discover the packages produced by the pack step; both the post-release hook args and @@ -241,13 +246,15 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) switch (hookUpdates.Count) { case 0: - reporter.Info("The post-release hook modified no files."); + reporter.Notice("The post-release hook modified no files."); break; case 1: - reporter.Info("The post-release hook modified 1 file."); + reporter.Notice("The post-release hook modified 1 file."); break; default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"The post-release hook modified {hookUpdates.Count} files.")); + reporter.Notice(string.Create( + CultureInfo.InvariantCulture, + $"The post-release hook modified {hookUpdates.Count} files.")); break; } } @@ -265,19 +272,21 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) switch (selfReferenceUpdates.Count) { case 0: - reporter.Info("No self-referenced files were modified."); + reporter.Notice("No self-referenced files were modified."); break; case 1: - reporter.Info("1 self-referenced file was modified."); + reporter.Notice("1 self-referenced file was modified."); break; default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"{selfReferenceUpdates.Count} self-referenced files were modified.")); + reporter.Notice(string.Create( + CultureInfo.InvariantCulture, + $"{selfReferenceUpdates.Count} self-referenced files were modified.")); break; } } else { - reporter.Info("Self-reference update skipped: option 'dogfood' is false."); + reporter.Notice("Self-reference update skipped: option 'dogfood' is false."); } // Assemble the post-release commit from the self-reference rewrites and the hook's changes. @@ -304,13 +313,17 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) var parts = line.Split('\t'); if (parts.Length != 3) { - reporter.Warning(string.Create(CultureInfo.InvariantCulture, $"Release asset list {path}, line #{i}: invalid line '{line}'")); + reporter.Warning(string.Create( + CultureInfo.InvariantCulture, + $"Release asset list {path}, line #{i}: invalid line '{line}'")); continue; } if (!File.Exists(parts[0])) { - reporter.Warning(string.Create(CultureInfo.InvariantCulture, $"Release asset list {path}, line #{i}: asset not found '{parts[0]}'")); + reporter.Warning(string.Create( + CultureInfo.InvariantCulture, + $"Release asset list {path}, line #{i}: asset not found '{parts[0]}'")); continue; } diff --git a/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs b/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs index d3042886..bf191831 100644 --- a/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs +++ b/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs @@ -33,12 +33,12 @@ public Task ExecuteAsync(CancellationToken cancellationToken) if (versionFile.ApplyChange(change)) { versionFile.Save(versioningSettings.PrereleaseTag); - reporter.Info($"Version spec changed from {previousSpec} to {versionFile.Spec}."); - reporter.Info($"Review and commit the modified {VersionFile.FileName} file."); + reporter.Notice($"Version spec changed from {previousSpec} to {versionFile.Spec}."); + reporter.Notice($"Review and commit the modified {VersionFile.FileName} file."); } else { - reporter.Info("Version spec not changed."); + reporter.Notice("Version spec not changed."); } return Task.FromResult(0); diff --git a/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs b/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs index 363bca88..f1d4f0a6 100644 --- a/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs +++ b/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs @@ -4,7 +4,6 @@ using System.ComponentModel; using System.Threading; using System.Threading.Tasks; -using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Infrastructure.Execution; using Buildvana.Tool.Services.Git; using Buildvana.Tool.Services.Versioning; @@ -12,7 +11,7 @@ namespace Buildvana.Tool.Subcommands; -[ImplementsCommand("version show | version", defaultVerbosity: Verbosity.Minimal)] +[ImplementsCommand("version show | version")] [Description("Show current and published version information.")] internal sealed class VersionShowCommand(VersionService version, GitService git, IAnsiConsole console) : IBvCommand { diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs new file mode 100644 index 00000000..d2e81e80 --- /dev/null +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs @@ -0,0 +1,39 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using Buildvana.Core.ConsoleOutput; + +internal sealed class MessageLevelExtensionsTests +{ + [Test] + [Arguments(MessageLevel.Error, Verbosity.Quiet)] + [Arguments(MessageLevel.Warning, Verbosity.Minimal)] + [Arguments(MessageLevel.Notice, Verbosity.Minimal)] + [Arguments(MessageLevel.Info, Verbosity.Normal)] + [Arguments(MessageLevel.Detail, Verbosity.Detailed)] + [Arguments(MessageLevel.Trace, Verbosity.Diagnostic)] + public async Task MinimumVerbosity_KnownLevel_ReturnsThreshold(MessageLevel level, Verbosity expected) + { + await Assert.That(level.MinimumVerbosity()).IsEqualTo(expected); + } + + [Test] + public async Task MinimumVerbosity_CoversEveryLevel() + { + // Guards the switch against a level added without a threshold: the arm would be missing, not wrong, + // and no per-level test above would notice. + foreach (var level in Enum.GetValues()) + { + await Assert.That(() => level.MinimumVerbosity()).ThrowsNothing(); + } + } + + [Test] + [Arguments(-1)] + [Arguments(42)] + public async Task MinimumVerbosity_UnknownLevel_Throws(int level) + { + var exception = await Assert.That(() => ((MessageLevel)level).MinimumVerbosity()).Throws(); + await Assert.That(exception?.ParamName).IsEqualTo("level"); + } +} diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs new file mode 100644 index 00000000..629bdc8d --- /dev/null +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs @@ -0,0 +1,75 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Text; +using Buildvana.Core.ConsoleOutput; +using Buildvana.Core.Testing; + +internal sealed class ReporterExtensionsTests +{ + private static readonly CompositeFormat Format = CompositeFormat.Parse("{0} in {1}"); + + [Test] + public async Task MessageShortcuts_ReportAtTheirOwnLevel() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + reporter.Error("boom"); + reporter.Warning("careful"); + reporter.Notice("noted"); + reporter.Info("working"); + reporter.Detail("details"); + reporter.Trace("trace"); + string[] expected = + [ + "error: boom", + "warning: careful", + "notice: noted", + "info: working", + "detail: details", + "trace: trace", + ]; + await Assert.That(reporter.ErrorText).IsEqualTo(string.Join(Environment.NewLine, expected) + Environment.NewLine); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task FormattingShortcuts_FormatArgumentsAndReportAtTheirOwnLevel() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + reporter.Error(Format, "boom", 1); + reporter.Warning(Format, "careful", 2); + reporter.Notice(Format, "noted", 3); + reporter.Info(Format, "working", 4); + reporter.Detail(Format, "details", 5); + reporter.Trace(Format, "trace", 6); + string[] expected = + [ + "error: boom in 1", + "warning: careful in 2", + "notice: noted in 3", + "info: working in 4", + "detail: details in 5", + "trace: trace in 6", + ]; + await Assert.That(reporter.ErrorText).IsEqualTo(string.Join(Environment.NewLine, expected) + Environment.NewLine); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task Report_DisabledLevel_WritesNothing() + { + using var reporter = new StringWriterReporter(Verbosity.Minimal); + reporter.Report(MessageLevel.Info, Format, "working", 1); + await Assert.That(reporter.ErrorText).IsEmpty(); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task Report_NullFormat_Throws() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + + // ReSharper disable once AccessToDisposedClosure // the assertion invokes the delegate before returning + await Assert.That(() => reporter.Report(MessageLevel.Info, (CompositeFormat)null!)).Throws(); + } +} diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs index 3009308a..3948bb2a 100644 --- a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs @@ -9,6 +9,7 @@ internal sealed class TextWriterReporterTests [Test] [Arguments(MessageLevel.Error, "error")] [Arguments(MessageLevel.Warning, "warning")] + [Arguments(MessageLevel.Notice, "notice")] [Arguments(MessageLevel.Info, "info")] [Arguments(MessageLevel.Detail, "detail")] [Arguments(MessageLevel.Trace, "trace")] @@ -22,6 +23,7 @@ public async Task Report_EnabledLevel_WritesLabeledLineToErrorWriter(MessageLeve [Test] [Arguments(Verbosity.Quiet, MessageLevel.Warning)] + [Arguments(Verbosity.Quiet, MessageLevel.Notice)] [Arguments(Verbosity.Minimal, MessageLevel.Info)] [Arguments(Verbosity.Normal, MessageLevel.Detail)] [Arguments(Verbosity.Detailed, MessageLevel.Trace)] @@ -33,6 +35,21 @@ public async Task Report_DisabledLevel_WritesNothing(Verbosity verbosity, Messag await Assert.That(reporter.OutputText).IsEmpty(); } + [Test] + [Arguments(MessageLevel.Error, Verbosity.Quiet)] + [Arguments(MessageLevel.Warning, Verbosity.Minimal)] + [Arguments(MessageLevel.Notice, Verbosity.Minimal)] + [Arguments(MessageLevel.Info, Verbosity.Normal)] + [Arguments(MessageLevel.Detail, Verbosity.Detailed)] + [Arguments(MessageLevel.Trace, Verbosity.Diagnostic)] + public async Task Report_AtLevelMinimumVerbosity_Writes(MessageLevel level, Verbosity verbosity) + { + using var reporter = new StringWriterReporter(verbosity); + reporter.Report(level, "something happened"); + await Assert.That(reporter.ErrorText).EndsWith($"something happened{Environment.NewLine}"); + await Assert.That(reporter.OutputText).IsEmpty(); + } + [Test] [Arguments(MessageLevel.Error, "\e[91m", "error")] [Arguments(MessageLevel.Warning, "\e[93m", "warning")] @@ -62,7 +79,10 @@ public async Task Report_UnknownLevel_Throws() using var reporter = new StringWriterReporter(Verbosity.Diagnostic); // ReSharper disable once AccessToDisposedClosure // the assertion invokes the delegate before returning - await Assert.That(() => reporter.Report((MessageLevel)(-1), "something happened")).Throws(); + var exception = await Assert.That(() => reporter.Report((MessageLevel)(-1), "something happened")) + .Throws(); + + await Assert.That(exception?.ParamName).IsEqualTo("level"); } [Test] diff --git a/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs b/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs index 6cb56751..1668e47f 100644 --- a/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs +++ b/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs @@ -44,8 +44,9 @@ public async Task Report_Warning_LogsBuildWarning() } [Test] - [Arguments(MessageLevel.Info, MessageImportance.High)] - [Arguments(MessageLevel.Detail, MessageImportance.Normal)] + [Arguments(MessageLevel.Notice, MessageImportance.High)] + [Arguments(MessageLevel.Info, MessageImportance.Normal)] + [Arguments(MessageLevel.Detail, MessageImportance.Low)] [Arguments(MessageLevel.Trace, MessageImportance.Low)] public async Task Report_MessageLevel_LogsMessageWithExpectedImportance( MessageLevel level, @@ -81,10 +82,13 @@ public async Task Verbosity_WithoutEngineServices_IsDiagnostic() await Assert.That(reporter.Verbosity).IsEqualTo(Verbosity.Diagnostic); } + // Both ends of this table over-claim on purpose: Low answers Diagnostic rather than Detailed so that + // formatted Trace messages are not short-circuited away under -v:diag, and an engine that logs nothing + // floors at Minimal rather than Quiet so that formatted warnings survive MSBuild's own quiet verbosity. [Test] [Arguments(MessageImportance.Low, Verbosity.Diagnostic)] - [Arguments(MessageImportance.Normal, Verbosity.Detailed)] - [Arguments(MessageImportance.High, Verbosity.Normal)] + [Arguments(MessageImportance.Normal, Verbosity.Normal)] + [Arguments(MessageImportance.High, Verbosity.Minimal)] [Arguments(null, Verbosity.Minimal)] public async Task Verbosity_WithEngineServices_TracksImportanceFiltering( MessageImportance? minimumImportance, @@ -193,7 +197,7 @@ public async Task ChildError_LogsLowImportanceMessage_NotABuildError() [Test] public async Task ChildLines_WhenEngineDiscardsLowImportance_AreNotForwarded() { - // Dropped twice over: the reporter's minimumVerbosity gate (verbosity here is Normal) and + // Dropped twice over: the reporter's minimumVerbosity gate (verbosity here is Minimal) and // TaskLoggingHelper's own importance filtering both consult the same EngineServices. var (reporter, engine) = CreateReporter(MessageImportance.High); reporter.ChildOutput("out", null); diff --git a/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs b/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs index f46bba73..174a435e 100644 --- a/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs +++ b/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs @@ -1,7 +1,6 @@ // Copyright (C) Tenacom and Contributors. Licensed under the MIT license. // See the LICENSE file in the project root for full license information. -using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Infrastructure.Execution; internal sealed class ImplementsCommandAttributeTests @@ -23,13 +22,6 @@ public async Task Aliases_AreNormalizedToLowercase() await Assert.That(string.Join(' ', attribute.AliasPaths[0])).IsEqualTo("version show"); } - [Test] - public async Task DefaultVerbosity_DefaultsToNormal() - { - var attribute = new ImplementsCommandAttribute("foo"); - await Assert.That(attribute.DefaultVerbosity).IsEqualTo(Verbosity.Normal); - } - [Test] [Arguments("")] [Arguments(" ")] diff --git a/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs new file mode 100644 index 00000000..5710d00e --- /dev/null +++ b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs @@ -0,0 +1,139 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +/// +/// End-to-end tests of what a successful release records at notice level: the lines a run at the +/// default verbosity shows, which are the whole account of the release a user gets. +/// +/// +/// Counted lines are asserted one repository shape at a time, singular and plural alike. Which shape +/// produces which wording is invisible from the code — the branches sit in the middle of a command that only +/// runs end to end — and a count that reads wrong is exactly the kind of thing that rots unnoticed. +/// The harness anchors the process's current directory and sets an environment variable, so these tests +/// cannot share the process with others that run at the same time. +/// +[NotInParallel] +internal sealed class ReleaseCommandReportingTests +{ + // Asking for a prerelease line that is already a prerelease line changes nothing, which is a decision + // worth recording: the release goes on, and the version file it leaves behind is the one it found. + [Test] + public async Task Release_WithBumpToUnstable_OnAPrereleaseLine_RecordsTheSpecUnchanged() + { + using var harness = new ReleaseHarness(new() { Dogfood = false }); + + var exitCode = await harness.RunAsync("--bump", "unstable").ConfigureAwait(false); + + await Assert.That(exitCode).IsEqualTo(0); + await Assert.That(harness.Notices).Contains("Version spec not changed."); + await Assert.That(harness.ReadFile("VERSION").Trim()).IsEqualTo("2.3-"); + } + + // Public API files are shipped in pairs - the unshipped file is emptied into the shipped one - so the + // count is always even, and the singular case the other counted lines have does not exist here. + [Test] + public async Task Release_ShippingPublicApis_CountsBothFilesOfThePair() + { + using var harness = new ReleaseHarness(new() + { + VersionSpec = "2.3", + Dogfood = false, + CheckPublicApi = false, + UnshippedPublicApi = "#nullable enable\nTest.Thing\n", + }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("2 public API files were modified."); + } + + [Test] + public async Task Release_WithoutPublicApis_RecordsThatNoneWereModified() + { + using var harness = new ReleaseHarness(new() { VersionSpec = "2.3", Dogfood = false }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("No public API files were modified."); + } + + // A hook that changes nothing is not a hook that did not run, and the two have to look different: + // the release still commits and publishes, and the line is what says the hook had its turn. + [Test] + public async Task Release_WithHookThatChangesNothing_RecordsNoModifiedFiles() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.AppRunner.Runs.Count).IsEqualTo(1); + await Assert.That(harness.Notices).Contains("The post-release hook modified no files."); + } + + [Test] + public async Task Release_WithHookThatChangesOneFile_RecordsItInTheSingular() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + // ReSharper disable once AccessToDisposedClosure // False positive: the hook runs before the harness is disposed + harness.HookBehavior = () => harness.WriteFile("docs/release-notes.md", "Released.\n"); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("The post-release hook modified 1 file."); + } + + [Test] + public async Task Release_WithHookThatChangesSeveralFiles_RecordsTheirNumber() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + // ReSharper disable once AccessToDisposedClosure // False positive: the hook runs before the harness is disposed + harness.HookBehavior = () => WriteTwoFiles(harness); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("The post-release hook modified 2 files."); + } + + // Dogfooding that rewrites nothing is the failure mode the self-reference update is most likely to have + // - a name that matches no reference rewrites just as silently as a repository that has none - so the + // line has to be there even when the answer is zero. + [Test] + public async Task Release_WithNoSelfReferenceTargets_RecordsThatNoneWereModified() + { + using var harness = new ReleaseHarness(new() { SelfReferenceTargets = 0 }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("No self-referenced files were modified."); + } + + [Test] + public async Task Release_WithOneSelfReferenceTarget_RecordsItInTheSingular() + { + using var harness = new ReleaseHarness(new() { SelfReferenceTargets = 1 }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("1 self-referenced file was modified."); + } + + [Test] + public async Task Release_WithEverySelfReferenceTarget_RecordsTheirNumber() + { + using var harness = new ReleaseHarness(); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("3 self-referenced files were modified."); + } + + // A method rather than a two-statement lambda: what the hook writes is captured from the harness, and + // a parameter keeps it out of a closure whose target is disposed by the time the analyzer looks at it. + private static void WriteTwoFiles(ReleaseHarness harness) + { + harness.WriteFile("docs/release-notes.md", "Released.\n"); + harness.WriteFile("docs/announcement.md", "Announcing the release.\n"); + } +} diff --git a/tests/Buildvana.Tool.Tests/ReleaseHarness.cs b/tests/Buildvana.Tool.Tests/ReleaseHarness.cs index 237ef681..2d567a1a 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseHarness.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseHarness.cs @@ -117,6 +117,13 @@ public ReleaseHarness(ReleaseHarnessOptions? options = null) /// public FakeFileBasedAppRunner AppRunner { get; } = new(); + /// + /// Gets the messages the release recorded at level, in order: the + /// record of what the release did, which is what a run at the default verbosity shows. + /// + public IEnumerable Notices + => Reporter.Messages.Where(x => x.Level == MessageLevel.Notice).Select(x => x.Message); + /// /// Gets the observable steps of the release, in order. /// @@ -317,14 +324,25 @@ private void PopulateRepository() private void WriteVersionFile() => Repo.WriteFile("VERSION", _options.VersionSpec + "\n"); // The three files the self-reference update rewrites, each carrying a reference to one of the packages - // this release produces, at the version released before this one. + // this release produces, at the version released before this one. Only the first + // ReleaseHarnessOptions.SelfReferenceTargets of them are written, so that a test can have the update + // find any number of them, none included. private void WriteSelfReferenceTargets() { + if (_options.SelfReferenceTargets < 1) + { + return; + } + var globalJson = new JsonObject { ["msbuild-sdks"] = new JsonObject { [ProducedPackageIds[0]] = PreviousVersion }, }; WriteFile("global.json", globalJson.ToJsonString(IndentedJson)); + if (_options.SelfReferenceTargets < 2) + { + return; + } var toolManifest = new JsonObject { @@ -340,6 +358,10 @@ private void WriteSelfReferenceTargets() }, }; WriteFile(".config/dotnet-tools.json", toolManifest.ToJsonString(IndentedJson)); + if (_options.SelfReferenceTargets < 3) + { + return; + } var packageVersions = $""" diff --git a/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs b/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs index 6e0f2f3d..a6b56f47 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs @@ -39,6 +39,14 @@ internal sealed record ReleaseHarnessOptions /// public bool Dogfood { get; init; } = true; + /// + /// Gets the number of self-reference targets the repository contains, out of the three the updater knows + /// about: global.json, .config/dotnet-tools.json, and Directory.Packages.props, in + /// that order. What is written is a prefix of that list: a repository legitimately has fewer than all + /// three, and one that references none of the packages it produces has none at all. + /// + public int SelfReferenceTargets { get; init; } = 3; + /// /// Gets the value of release.checkPublicApi. /// diff --git a/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs b/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs new file mode 100644 index 00000000..b6c10544 --- /dev/null +++ b/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs @@ -0,0 +1,190 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Globalization; +using Buildvana.Tool.Services.ServerAdapters; + +/// +/// Tests of driven directly, over the same repository and service graph a real +/// release runs on (see ). +/// +/// +/// The release command walks one path through this class; the class is a contract for every adapter, +/// so what it refuses and what it records are asserted here rather than through the one caller that exists +/// today. The order its methods may be called in is part of that contract: a commit after a push, or a +/// post-release commit before the release commit, would leave a release the rollback cannot undo, and each +/// guard is what turns that into an error at the call that got it wrong. +/// The harness anchors the process's current directory and sets an environment variable, so these tests +/// cannot share the process with others that run at the same time. +/// +[NotInParallel] +internal sealed class ServerReleaseTests +{ + [Test] + public async Task EnsureReleaseCommit_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + + void Act() => release.EnsureReleaseCommit(); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task UpdateRepository_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.UpdateRepository("docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task UpdateRepository_AfterAPostReleaseCommit_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + AddPostReleaseCommit(harness, release, 1); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.UpdateRepository("docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("after a post-release commit"); + } + } + + [Test] + public async Task AddPostReleaseCommit_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.AddPostReleaseCommit("Post-release updates [skip ci]", "docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task AddPostReleaseCommit_BeforeTheReleaseCommit_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + harness.WriteFile("docs/early.md", "Too early.\n"); + + void Act() => release.AddPostReleaseCommit("Post-release updates [skip ci]", "docs/early.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("before the release commit"); + } + } + + // Nothing to push is not an error - a release may legitimately change no file at all - so it is recorded + // and the release goes on. The line matters because the push is otherwise invisible: without it, a run + // that pushed nothing and a run that pushed the release commit read exactly alike. + [Test] + public async Task PushUpdates_WithoutAReleaseCommit_RecordsThatThereIsNothingToPush() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.PushUpdates(); + } + + await Assert.That(harness.Notices).Contains("Repository unchanged, no commit to push."); + await Assert.That(harness.Repo.GetRemoteTipSha()).IsNull(); + } + + [Test] + [Arguments(0, "with no assets.")] + [Arguments(1, "with 1 asset.")] + [Arguments(2, "with 2 assets.")] + public async Task PublishAsync_RecordsTheNumberOfAssets(int assetCount, string expectedEnding) + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + for (var i = 0; i < assetCount; i++) + { + release.AddAsset(string.Create(CultureInfo.InvariantCulture, $"artifacts/asset-{i}.bin")); + } + + await release.PublishAsync().ConfigureAwait(false); + } + + await Assert.That(harness.Notices).Contains($"Published release {harness.ComputeVersion()} {expectedEnding}"); + } + + // Every commit the release made is walked back by one rollback action, and recorded by one line naming + // how many: the release commit is always there, the post-release commits are however many the caller + // added. The release command adds at most one of them; the class allows any number, so the ladder is + // asserted here rather than through the one caller. + [Test] + [Arguments(0, "Undid the release commit.")] + [Arguments(1, "Undid the release commit and 1 post-release commit.")] + [Arguments(2, "Undid the release commit and 2 post-release commits.")] + public async Task DisposeAsync_RecordsEveryUndoneCommitAtOnce(int postReleaseCommits, string expectedNotice) + { + using var harness = new ReleaseHarness(); + var initialSha = harness.Repo.HeadSha; + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + for (var i = 0; i < postReleaseCommits; i++) + { + AddPostReleaseCommit(harness, release, i + 1); + } + } + + await Assert.That(harness.Notices).Contains(expectedNotice); + await Assert.That(harness.Repo.GetCommits(1)[0].Sha).IsEqualTo(initialSha); + } + + // A release object over the harness's repository, with a committer identity for the commits it makes: + // the command sets one from the server adapter before creating the release, and nothing else does. + private static async Task StartReleaseAsync(ReleaseHarness harness) + { + harness.Repo.SetCommitterIdentity("Buildvana Test Bot", "bot@buildvana.invalid"); + return await harness.Adapter.CreateReleaseAsync().ConfigureAwait(false); + } + + private static void AddPostReleaseCommit(ReleaseHarness harness, ServerRelease release, int ordinal) + { + var path = string.Create(CultureInfo.InvariantCulture, $"docs/post-release-{ordinal}.md"); + harness.WriteFile(path, string.Create(CultureInfo.InvariantCulture, $"Post-release change #{ordinal}.\n")); + release.AddPostReleaseCommit( + string.Create(CultureInfo.InvariantCulture, $"Post-release updates #{ordinal} [skip ci]"), + path); + } +}