Skip to content

[Enhancement]: Cross-app — four breaks of the code's own rules: hand-built CLI JSON, intervals on the wall clock, a second CSV escaper, an object lock #2855

Open

Description

@laurentiu021

Problem

  1. The command line's fault path builds JSON by hand. Every --json payload goes through CliRunner.Json (System.Text.Json), and Execute_UnknownJson_EmitsParseableJsonError pins that --json output always parses. App.RunCliAndExit's unexpected-fault branch instead builds {"error":"..."} by interpolation and escapes only \ and ", so a message with a CR, LF or TAB prints a document that ConvertFrom-Json rejects, on the branch whose comment says it keeps the output machine-readable.
  2. Intervals measured on the wall clock. EtwBandwidthSource and TemperatureService write the rule down: interval arithmetic uses TimeProvider.GetTimestamp/GetElapsedTime, because the clock jumps. Four places still subtract DateTime values:
    • the tray's 4-hour RAM, uptime and disk-warning cooldowns (DateTime.UtcNow), so a clock set back keeps them silent for longer, and the cooldown cannot be tested, which is why the tray's threshold tests only assert DoesNotThrow;
    • Process Manager's CPU window around Thread.Sleep(100);
    • Ping's sample stamps (DateTime.UtcNow), next to the TimeProvider the service already takes for its delay;
    • Bandwidth Monitor's 5-second history throttle, which subtracts local DateTime.Now values, so during the repeated autumn hour no sample is saved while the tab is open.
  3. A second CSV escaper. Csv.cs says "the escaping lives here once rather than being re-derived per tab", and nine files export through it, but LogsViewModel keeps its own private Csv(string?). They agree today; the next rule change, such as the formula guard [Bug]: Cross-app — CSV exports have no UTF-8 BOM, so Excel garbles accented names and dashes, and a field starting with = is read as a formula #2794 asks for, would have to be made twice.
  4. The last object lock. The app's locks are System.Threading.Lock, and EtaCalculator._gate is the one private readonly object left, used only in lock statements.

Proposed solution

  • CliRunner.ErrorJson(message) through the same serializer, with a test that a message holding CR, LF and TAB round-trips through JsonDocument.Parse.
  • TimeProvider timestamps for the four intervals, through the optional-parameter seam PingMonitorService already has, with a fake-clock test for the tray cooldown; DateTime.Now stays only for values that are shown or saved.
  • LogsViewModel calls Helpers.Csv.Field, and the test that reads the private method by reflection moves to it.
  • private readonly Lock _gate = new();, and a guard that no object field is used as a lock target.

Affected tab

Command line, notification area, Process Manager, Ping, Bandwidth Monitor, System Logs.

Activity

  1. laurentiu021 commented on Oct 21, 2026

    @laurentiu021
    OwnerAuthor

    Point 3 is done in 1.130.33 (#2920): LogsViewModel no longer has its own Csv(string?). Its export builds rows with Csv.AppendRow and writes them with Csv.Write, and the test that read the private method by reflection now tests the export itself in LogsViewModelExtendedTests. Points 1, 2 and 4 are still open.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions