Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b4005c84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var duplicate = document.TestCases.GroupBy(item => item.TestCaseId).FirstOrDefault(group => group.Key == Guid.Empty || group.Count() > 1); | ||
| if (duplicate is not null) | ||
| throw new InvalidDataException("Test-case identities must be non-empty and unique."); | ||
| foreach (var testCase in document.TestCases) |
There was a problem hiding this comment.
Reject undefined result values before computing disposition
Because JsonStringEnumConverter accepts integer enum tokens by default, a workspace containing "result": 99 passes this validation. If every other test passes, the undefined result is counted as neither NotRun nor blocking, so IsComplete becomes true and the audit report falsely declares the package complete; validate every result with Enum.IsDefined before accepting the document.
Useful? React with 👍 / 👎.
| var source = Path.GetFullPath(evidence.SourcePath); | ||
| if (!File.Exists(source)) | ||
| throw new FileNotFoundException($"Evidence file '{evidence.DisplayName}' is missing.", source); | ||
| var bytes = await File.ReadAllBytesAsync(source, cancellationToken); |
There was a problem hiding this comment.
Stream evidence files while building the archive
When a workspace contains several large evidence files, this reads each entire file into a new byte array and retains every array in entries until ZIP creation begins. The documented limits permit 256 MB per file and 100 files per test, so even a handful of valid captures can exhaust process memory and make export fail; hash and copy each file directly into its archive entry instead of buffering the complete evidence set.
Useful? React with 👍 / 👎.
| private void New_Click(object sender, RoutedEventArgs e) | ||
| => ApplyDocument(_workspaceService.CreateDefault(), null); |
There was a problem hiding this comment.
Confirm before replacing an unsaved workspace
After an operator edits outcomes, notes, or evidence references without saving, clicking New immediately replaces the current document and rows with a default workspace. There is no dirty-state check or confirmation, so potentially lengthy FAT/SAT execution records are irrecoverably discarded; prompt to save or cancel before applying the replacement document.
Useful? React with 👍 / 👎.
| throw new InvalidDataException($"Unsupported FAT/SAT schema version {document.SchemaVersion}. Expected {FatSatWorkspaceDocument.CurrentSchemaVersion}."); | ||
| if (document.WorkspaceId == Guid.Empty) | ||
| throw new InvalidDataException("Workspace identity is missing."); | ||
| if (document.TestCases.Count > 1000) |
There was a problem hiding this comment.
Reject null collections as malformed workspace data
Opening otherwise valid JSON containing "testCases": null makes deserialization assign null despite the property initializer, and this dereference throws NullReferenceException rather than the intended InvalidDataException; similarly, null test-case elements or evidence collections fail later. These exceptions bypass Open_Click's normal malformed-file error handling, so validate collection and element nullability before accessing them.
Useful? React with 👍 / 👎.
|
Closing as superseded by the dedicated IO List Testing direction agreed for ARSAS. This PR is also stacked on PR #109, which was closed unmerged after evidence-correctness findings, so retargeting it to |
Purpose
Implement ARSAS P2 as a unified, schema-versioned FAT/SAT Test & Evidence Workspace rather than another disconnected protocol viewer.
Workspace
NotRun,Pass,Fail,Review,Blocked,NotApplicableDefault IEC 61850 plan
The initial plan covers:
Evidence integrity
Persistence and audit package
*.arsas-fat.json.partialplus atomic moveworkspace.jsonwith package-relative evidence pathsreport.mdwith scope, disposition, outcomes, deviations, and evidence referencesevidence/...immutable source filesSHA256SUMS.txtAcceptance boundary
A package is COMPLETE only when no test remains
NotRunand no test isFail,Review, orBlocked. Operator-entered outcomes remain operator-owned. The workspace does not claim formal conformance, universal interoperability, calibrated measurement, or authorization for live control.Final automated validation at head
47b4005c8484c594e03ba4e49712d15c32bf94e4ARSAS-win-x64ARSAS-test-evidenceARSAS-source-snapshotThe installer workflow did not run for this stacked delta because no installer, release, version, project, or engine-lock path changed. The full WPF application and portable package were compiled from the P2 head.
Regression coverage
Stack boundary
This PR is intentionally based on P1 branch
agent/p1-sv-evidence-bundle/ PR #109 and contains only the P2 delta. Public stable release remains 1.6.18.