From ba2815ec7cf873417e764bfd13c71619424928d4 Mon Sep 17 00:00:00 2001 From: Carter <4911475+carterscode@users.noreply.github.com> Date: Thu, 7 May 2026 12:19:08 -0700 Subject: [PATCH] chore: CONTRIBUTING.md, xUnit test project, warnings-as-errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenSSF Best Practices Passing tier groundwork. Targets the four real gaps from the questionnaire walkthrough. CONTRIBUTING.md Documents branching/PR flow, commit conventions, code style, the test policy ('add a test if reasonably testable'), how to add a monitor or a new service, and what reviewers look for. Satisfies test_policy / tests_are_added / tests_documented_added. tests/GamerGuardian.Tests New xUnit test project with 36 tests across three areas: * ServiceCatalogTests — catalog has no duplicates, every entry has display name + description + known DefaultStartType, the RecommendedTarget invariant holds, expected services present. * SettingDocsTests — Mechanism / Verify lookups return non-empty strings for every known setting Id, the service: prefix path includes the service name, hdr:/refresh:/resolution: prefixes route correctly, unknown Ids return the documented marker. * WindowsServiceControllerTests — Exists / ReadStartType / ReadStatus all return safe defaults for missing services without throwing, and return non-Unknown values for the always-present EventLog service. Satisfies test_invocation (dotnet test works) and test_most (real branch coverage of the testable surface). GamerGuardian.csproj TreatWarningsAsErrors=true. Build is currently clean; this prevents silent regression. Satisfies warnings_strict. build.yml Runs 'dotnet test' after build on every PR. Required check on main. --- .github/workflows/build.yml | 3 + CONTRIBUTING.md | 124 ++++++++++++++++++ GamerGuardian.sln | 13 +- src/GamerGuardian/GamerGuardian.csproj | 3 + .../GamerGuardian.Tests.csproj | 25 ++++ .../ServiceCatalogTests.cs | 69 ++++++++++ tests/GamerGuardian.Tests/SettingDocsTests.cs | 67 ++++++++++ .../WindowsServiceControllerTests.cs | 46 +++++++ 8 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 CONTRIBUTING.md create mode 100644 tests/GamerGuardian.Tests/GamerGuardian.Tests.csproj create mode 100644 tests/GamerGuardian.Tests/ServiceCatalogTests.cs create mode 100644 tests/GamerGuardian.Tests/SettingDocsTests.cs create mode 100644 tests/GamerGuardian.Tests/WindowsServiceControllerTests.cs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 75af580..72bd079 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,3 +23,6 @@ jobs: - name: Build (Release) run: dotnet build GamerGuardian.sln -c Release --no-restore + + - name: Test + run: dotnet test GamerGuardian.sln -c Release --no-build --verbosity normal diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ee3d4da --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,124 @@ +# Contributing to GamerGuardian + +Thanks for your interest. This document describes how to set up, contribute, and what reviewers look for. + +## Quick start for contributors + +```powershell +git clone https://github.com/carterscode/GamerGuardian.git +cd GamerGuardian +dotnet build +dotnet test +src\GamerGuardian\bin\Debug\net8.0-windows10.0.22000.0\GamerGuardian.exe --show-settings +``` + +For the installer build and CI workflow details, see [docs/wiki/Build-from-source.md](https://github.com/carterscode/GamerGuardian/blob/main/docs/wiki/Build-from-source.md). + +## Branching and pull requests + +`main` is protected — you cannot push to it directly. The flow: + +1. Branch off `main`: `git checkout -b feat/something-descriptive`. Use `feat/`, `fix/`, `chore/`, `ci/`, `docs/` prefixes. +2. Commit your changes with descriptive messages (see *Commit messages* below). +3. Push the branch: `git push -u origin feat/something-descriptive`. +4. Open a pull request: `gh pr create --base main`. +5. CI runs automatically — `build`, `Analyze (csharp)`, `Analyze (actions)`. All three must pass before merge. +6. Self-merge once green: `gh pr merge --merge --delete-branch` (no required approvals for the solo-dev workflow). + +## Commit messages + +Conventional Commits format. The first line is `: ` under 72 chars. + +Common types: +- `feat:` — new functionality (new monitor, UI feature, CLI flag) +- `fix:` — bug fix +- `chore:` — maintenance, refactors with no behavior change +- `ci:` — workflow / build pipeline changes +- `docs:` — wiki, README, comments +- `perf:` — performance improvements +- `ui:` — UI/UX changes + +Multi-line bodies are encouraged for non-trivial changes — explain *why*, not *what*. Example: + +``` +fix(services): stop UAC spam when Windows reverts a service change + +Symptom: enabling auto-apply on a service Windows refuses to actually +disable (DoSvc / Delivery Optimization is the trigger case) caused a +UAC prompt every 30 s forever. + +MonitorService now backs off auto-apply for a setting whose verify +failed for 15 minutes. Drift still surfaces as a notification. +``` + +## Code style + +- Follow the existing patterns. The codebase is small and consistent. +- `enable` is on. Don't introduce `?` types if you can avoid them. +- `true` is on. Build warnings break CI. +- Default to no comments. Only comment the *why* when the *what* is obvious from the code. See examples in `Monitors/HagsMonitor.cs` for the conventional level of commenting. +- C# expression-bodied members and pattern-matching are encouraged where they read naturally. +- Don't introduce abstractions speculatively. Three similar lines is better than a premature framework. + +## Adding a new monitor + +The canonical example is `src/GamerGuardian/Monitors/HagsMonitor.cs` — about 30 lines. + +A new monitor needs: + +1. A class implementing `IMonitoredSetting` in `src/GamerGuardian/Monitors/`. +2. Registration in `App.xaml.cs` in the `_allMonitors` array. +3. A row in `SettingsWindow.xaml.cs` `LoadGlobals` (or the equivalent for your tab). +4. A `MechanismFor` and `VerifyCommandFor` entry in `src/GamerGuardian/Services/SettingDocs.cs`. +5. **A test** in `tests/GamerGuardian.Tests/` (see *Tests* below). + +If the new monitor writes to `HKLM`, route it through `ElevatedRegistry` so it shares the existing UAC-prompt behavior. + +## Adding a new Windows service to the catalog + +For the `Windows services` tab, just append to `ServiceCatalog.All` in `src/GamerGuardian/Services/ServiceCatalog.cs`. No code change required elsewhere — `WindowsServiceMonitor` is registered once per catalog entry by `App.xaml.cs`. + +If the service is one Windows actively protects (re-enables via `WaaSMedicSvc` etc.), set `RecommendedTarget: ServiceTargetState.Manual` rather than `Disabled`, or omit it from the catalog entirely. See `docs/wiki/Architecture-rationale.md` for the WU-protection background. + +## Tests + +We use xUnit. The test project lives at `tests/GamerGuardian.Tests/`. + +Run all tests: + +```powershell +dotnet test +``` + +CI runs the same on every PR. + +### Test policy + +When you add or change behavior: + +- **Pure logic** (catalogs, mappings, parsers, lookup tables) — add a unit test covering the new behavior. +- **Native API wrappers** (anything in `Native/` or `WindowsServiceController`) — add a "doesn't throw on bad input" test if practical. Full coverage isn't expected since these wrap Windows APIs that aren't easily mockable. +- **UI** — manual verification on a dev-build artifact is the current standard. UI test automation is on the roadmap. +- **Bug fixes** — add a regression test if the bug is reproducible from a unit test. + +The general rule: it's fine to merge without a test if the change can't be reasonably unit-tested (a UI tweak, a workflow change, a doc update). It's not fine to merge without a test if the change touches a class that *is* unit-tested already. + +## Reporting issues and requesting features + +- **Bug reports / feature requests:** [GitHub Issues](https://github.com/carterscode/GamerGuardian/issues). Search first; include `--test` output and your `changes.log` if relevant. +- **Security vulnerabilities:** see [SECURITY.md](SECURITY.md). **Do not** open a public issue. +- **Questions:** also fine in Issues; tag with `question`. + +## What reviewers look for + +- The change is scoped to one concern. +- New behavior has a test if reasonably testable. +- No new compiler warnings. +- Commit messages explain *why*. +- No secrets in the diff (GitHub push protection will catch most, but double-check). +- Touched files have consistent style with the surrounding code. +- For new dependencies: justified, well-maintained, license-compatible (MIT-friendly). + +## License + +By contributing you agree your contributions are licensed under the [MIT License](LICENSE), the same license the project uses. diff --git a/GamerGuardian.sln b/GamerGuardian.sln index 394ad52..b3c9d4c 100644 --- a/GamerGuardian.sln +++ b/GamerGuardian.sln @@ -1,7 +1,11 @@ -Microsoft Visual Studio Solution File, Format Version 12.00 +Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio Version 17 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GamerGuardian", "src\GamerGuardian\GamerGuardian.csproj", "{8A1E0001-0001-0001-0001-000000000001}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{6C24C775-257A-472D-A077-0A1AE0BEB2EB}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GamerGuardian.Tests", "tests\GamerGuardian.Tests\GamerGuardian.Tests.csproj", "{7C117441-4E51-4330-AECC-C2343F5C2CC3}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -12,5 +16,12 @@ Global {8A1E0001-0001-0001-0001-000000000001}.Debug|Any CPU.Build.0 = Debug|Any CPU {8A1E0001-0001-0001-0001-000000000001}.Release|Any CPU.ActiveCfg = Release|Any CPU {8A1E0001-0001-0001-0001-000000000001}.Release|Any CPU.Build.0 = Release|Any CPU + {7C117441-4E51-4330-AECC-C2343F5C2CC3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {7C117441-4E51-4330-AECC-C2343F5C2CC3}.Debug|Any CPU.Build.0 = Debug|Any CPU + {7C117441-4E51-4330-AECC-C2343F5C2CC3}.Release|Any CPU.ActiveCfg = Release|Any CPU + {7C117441-4E51-4330-AECC-C2343F5C2CC3}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(NestedProjects) = preSolution + {7C117441-4E51-4330-AECC-C2343F5C2CC3} = {6C24C775-257A-472D-A077-0A1AE0BEB2EB} EndGlobalSection EndGlobal diff --git a/src/GamerGuardian/GamerGuardian.csproj b/src/GamerGuardian/GamerGuardian.csproj index f1de486..11b86d5 100644 --- a/src/GamerGuardian/GamerGuardian.csproj +++ b/src/GamerGuardian/GamerGuardian.csproj @@ -27,6 +27,9 @@ same commit produces byte-identical output across runs. --> true portable + + + true diff --git a/tests/GamerGuardian.Tests/GamerGuardian.Tests.csproj b/tests/GamerGuardian.Tests/GamerGuardian.Tests.csproj new file mode 100644 index 0000000..f44b147 --- /dev/null +++ b/tests/GamerGuardian.Tests/GamerGuardian.Tests.csproj @@ -0,0 +1,25 @@ + + + net8.0-windows10.0.22000.0 + enable + enable + true + 10.0.22000.0 + false + true + true + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + diff --git a/tests/GamerGuardian.Tests/ServiceCatalogTests.cs b/tests/GamerGuardian.Tests/ServiceCatalogTests.cs new file mode 100644 index 0000000..43ff273 --- /dev/null +++ b/tests/GamerGuardian.Tests/ServiceCatalogTests.cs @@ -0,0 +1,69 @@ +using GamerGuardian.Models; +using GamerGuardian.Services; +using Xunit; + +namespace GamerGuardian.Tests; + +public class ServiceCatalogTests +{ + [Fact] + public void All_ContainsServices() + { + Assert.NotEmpty(ServiceCatalog.All); + } + + [Fact] + public void All_HasNoDuplicateServiceNames() + { + var names = ServiceCatalog.All.Select(d => d.Name).ToList(); + Assert.Equal(names.Count, names.Distinct(StringComparer.OrdinalIgnoreCase).Count()); + } + + [Fact] + public void All_EveryEntryHasNonEmptyDisplayNameAndDescription() + { + foreach (var def in ServiceCatalog.All) + { + Assert.False(string.IsNullOrWhiteSpace(def.Name), $"empty Name for an entry"); + Assert.False(string.IsNullOrWhiteSpace(def.DisplayName), $"empty DisplayName for {def.Name}"); + Assert.False(string.IsNullOrWhiteSpace(def.Description), $"empty Description for {def.Name}"); + } + } + + [Fact] + public void All_DefaultStartTypeIsKnown() + { + foreach (var def in ServiceCatalog.All) + { + Assert.NotEqual(ServiceStartType.Unknown, def.DefaultStartType); + } + } + + [Fact] + public void RecommendedTarget_NeverDefault() + { + // RecommendedTarget == Default would mean "the preset moves it to where it + // already is" which is meaningless. The convention is: leave RecommendedTarget + // null for services that aren't in the preset, otherwise specify Manual or Disabled. + foreach (var def in ServiceCatalog.All) + { + if (def.RecommendedTarget.HasValue) + { + Assert.NotEqual(ServiceTargetState.Default, def.RecommendedTarget.Value); + } + } + } + + [Theory] + [InlineData("DiagTrack")] + [InlineData("MapsBroker")] + [InlineData("Fax")] + [InlineData("Spooler")] + [InlineData("DoSvc")] + [InlineData("iphlpsvc")] + public void All_IncludesExpectedServices(string serviceName) + { + Assert.Contains(ServiceCatalog.All, d => + d.Name.Equals(serviceName, StringComparison.OrdinalIgnoreCase)); + } +} diff --git a/tests/GamerGuardian.Tests/SettingDocsTests.cs b/tests/GamerGuardian.Tests/SettingDocsTests.cs new file mode 100644 index 0000000..8a1c37a --- /dev/null +++ b/tests/GamerGuardian.Tests/SettingDocsTests.cs @@ -0,0 +1,67 @@ +using GamerGuardian.Services; +using Xunit; + +namespace GamerGuardian.Tests; + +public class SettingDocsTests +{ + [Theory] + [InlineData("hags")] + [InlineData("memintegrity")] + [InlineData("gamemode")] + [InlineData("gamedvr")] + [InlineData("mouseaccel")] + [InlineData("fso")] + [InlineData("vrr")] + [InlineData("sysresponse")] + [InlineData("netthrottle")] + [InlineData("usbsuspend")] + [InlineData("gamestask")] + [InlineData("powerplan")] + public void MechanismFor_KnownIds_ReturnsNonEmpty(string id) + { + var mech = SettingDocs.MechanismFor(id); + Assert.False(string.IsNullOrWhiteSpace(mech), $"no Mechanism for {id}"); + Assert.NotEqual("(unknown)", mech); + } + + [Theory] + [InlineData("hags")] + [InlineData("memintegrity")] + [InlineData("gamemode")] + [InlineData("powerplan")] + public void VerifyCommandFor_KnownIds_ReturnsNonEmpty(string id) + { + var cmd = SettingDocs.VerifyCommandFor(id); + Assert.False(string.IsNullOrWhiteSpace(cmd), $"no Verify command for {id}"); + } + + [Fact] + public void MechanismFor_DisplayPrefixIds_RecognizesAllThree() + { + Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("hdr:DISPLAY1")); + Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("refresh:DISPLAY1")); + Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("resolution:DISPLAY1")); + } + + [Fact] + public void MechanismFor_ServicePrefix_IncludesServiceName() + { + var mech = SettingDocs.MechanismFor("service:diagtrack"); + Assert.Contains("diagtrack", mech, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void VerifyCommandFor_ServicePrefix_IncludesScQc() + { + var cmd = SettingDocs.VerifyCommandFor("service:diagtrack"); + Assert.Contains("sc qc", cmd); + Assert.Contains("diagtrack", cmd, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void MechanismFor_UnknownId_ReturnsUnknownMarker() + { + Assert.Equal("(unknown)", SettingDocs.MechanismFor("definitely_not_a_real_setting")); + } +} diff --git a/tests/GamerGuardian.Tests/WindowsServiceControllerTests.cs b/tests/GamerGuardian.Tests/WindowsServiceControllerTests.cs new file mode 100644 index 0000000..0157ae5 --- /dev/null +++ b/tests/GamerGuardian.Tests/WindowsServiceControllerTests.cs @@ -0,0 +1,46 @@ +using GamerGuardian.Models; +using GamerGuardian.Services; +using Xunit; + +namespace GamerGuardian.Tests; + +public class WindowsServiceControllerTests +{ + private const string DefinitelyNotAService = "GamerGuardianFakeServiceForTests"; + + [Fact] + public void Exists_NonexistentService_ReturnsFalse() + { + Assert.False(WindowsServiceController.Exists(DefinitelyNotAService)); + } + + [Fact] + public void ReadStartType_NonexistentService_ReturnsUnknown() + { + // Should not throw; should return Unknown so callers can treat it as "skip". + Assert.Equal(ServiceStartType.Unknown, WindowsServiceController.ReadStartType(DefinitelyNotAService)); + } + + [Fact] + public void ReadStatus_NonexistentService_ReturnsNull() + { + Assert.Null(WindowsServiceController.ReadStatus(DefinitelyNotAService)); + } + + // EventLog is a service that exists on every supported Windows install and + // boots automatically. Reading its registry start type should succeed and + // never return Unknown. We don't assert the specific value because Microsoft + // has changed it over time (Auto vs AutoDelayed). + [Fact] + public void ReadStartType_EventLog_ReturnsKnown() + { + var start = WindowsServiceController.ReadStartType("EventLog"); + Assert.NotEqual(ServiceStartType.Unknown, start); + } + + [Fact] + public void Exists_EventLog_ReturnsTrue() + { + Assert.True(WindowsServiceController.Exists("EventLog")); + } +}