From 0c60d4c677c78be9765fb3c1cac33e31785cf581 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 00:13:00 +0900 Subject: [PATCH 1/3] Document run-tests XmlPath as null when no XML is saved The skill said XmlPath is an empty string when no XML was saved, but every response that saves none sends null: all producers pass null and the response serializer keeps nulls. An agent that follows the docs and checks for an empty string misses the no-XML case (#3025). --- .agents/skills/uloop-run-tests/SKILL.md | 2 +- .claude/skills/uloop-run-tests/SKILL.md | 2 +- Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-run-tests/SKILL.md b/.agents/skills/uloop-run-tests/SKILL.md index a35cd0179..767a50003 100644 --- a/.agents/skills/uloop-run-tests/SKILL.md +++ b/.agents/skills/uloop-run-tests/SKILL.md @@ -58,7 +58,7 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string): Path to NUnit XML result file. Empty string when no XML was saved (typically on `Success: true`); populated only when tests failed or were inconclusive and the XML file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. diff --git a/.claude/skills/uloop-run-tests/SKILL.md b/.claude/skills/uloop-run-tests/SKILL.md index a35cd0179..767a50003 100644 --- a/.claude/skills/uloop-run-tests/SKILL.md +++ b/.claude/skills/uloop-run-tests/SKILL.md @@ -58,7 +58,7 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string): Path to NUnit XML result file. Empty string when no XML was saved (typically on `Success: true`); populated only when tests failed or were inconclusive and the XML file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md index a35cd0179..767a50003 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md @@ -58,7 +58,7 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string): Path to NUnit XML result file. Empty string when no XML was saved (typically on `Success: true`); populated only when tests failed or were inconclusive and the XML file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. From 0385785511c595f8044207b0082bf3253b1d454b Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 00:13:46 +0900 Subject: [PATCH 2/3] Report suites that fail outside their tests in run-tests results When a fixture's OneTimeTearDown throws, or a setup fixture's does, Unity marks that suite Failed while its tests keep Passed. run-tests judged the run from the tests alone, so it reported Status Passed and Success true, dropped the exception message, and saved no XML (#3026). - List the deepest Failed suite that has no failed child suite and no failed test in a new FailedSuites field, with its message and source location, and report the run as Failed with HasFailures true - Save the XML for such a run, with a on the suite element and a Failed run result - Move the skill's XML section to references/xml-results.md so SKILL.md stays under the 8,000-byte cap --- .agents/skills/uloop-run-tests/SKILL.md | 16 +- .../uloop-run-tests/references/xml-results.md | 13 ++ .claude/skills/uloop-run-tests/SKILL.md | 16 +- .../uloop-run-tests/references/xml-results.md | 13 ++ .../Editor/RunTestsResponseContractTests.cs | 108 +++++++++ .../RunTestsTestFrameworkResultTests.cs | 212 +++++++++++++++++- .../RunTests/RunTestsResponse.cs | 6 + .../RunTests/RunTestsResponseFactory.cs | 5 + .../FirstPartyTools/RunTests/Skill/SKILL.md | 16 +- .../RunTests/Skill/references.meta | 8 + .../RunTests/Skill/references/xml-results.md | 13 ++ .../Skill/references/xml-results.md.meta | 7 + .../TestFramework/NUnitXmlResultExporter.cs | 13 +- .../SerializableTestResultConverter.cs | 79 ++++++- .../TestRunner/SerializableTestResult.cs | 6 + 15 files changed, 494 insertions(+), 37 deletions(-) create mode 100644 .agents/skills/uloop-run-tests/references/xml-results.md create mode 100644 .claude/skills/uloop-run-tests/references/xml-results.md create mode 100644 Packages/src/Editor/FirstPartyTools/RunTests/Skill/references.meta create mode 100644 Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md create mode 100644 Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md.meta diff --git a/.agents/skills/uloop-run-tests/SKILL.md b/.agents/skills/uloop-run-tests/SKILL.md index 767a50003..c94853488 100644 --- a/.agents/skills/uloop-run-tests/SKILL.md +++ b/.agents/skills/uloop-run-tests/SKILL.md @@ -6,7 +6,7 @@ description: "Run Unity Test Runner and report detailed results. Use for EditMod # uloop run-tests -Execute Unity Test Runner. When tests fail or end inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. +Execute Unity Test Runner. When a test or suite fails or a test ends inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. `uloop run-tests` automatically compiles pending script changes before running tests. Pass `--skip-compile` only while validating active hot-reload patches, because the compile clears those patches; otherwise let the default compile surface errors and run against current scripts. `--skip-compile` skips only the CLI-side compile: Unity still imports script edits saved since the last compile, and that import reloads the domain as soon as the run releases its assembly lock, discarding active patches and ending the request. @@ -46,9 +46,9 @@ exact matches the full test name (Namespace.Class.Method). class runs every test Returns JSON with: -- `Success` (boolean): Whether every test passed or was skipped; `false` when any failed or was inconclusive +- `Success` (boolean): Whether every test passed or was skipped; `false` when a test or suite failed or a test was inconclusive - `Status` (string): Machine-readable execution status such as `Passed`, `Failed`, `Inconclusive`, `NoTestsFound`, or `ExecutionFailed` -- `HasFailures` (boolean): Whether any discovered test failed +- `HasFailures` (boolean): Whether any discovered test or suite failed - `Message` (string): Summary message - `NoTestsFound` (boolean): Whether Unity Test Runner discovered zero matching tests - `NoTestsFoundExplanation` (string): Agent-facing explanation when `NoTestsFound` is true; empty otherwise @@ -58,19 +58,15 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when a test or suite failed or a test was inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. ### XML Result File -When tests fail or end inconclusive, NUnit XML results are automatically saved to `{project_root}/.uloop/outputs/TestResults/.xml`. The XML contains per-test-case results including: - -- Test name and full name -- Pass/fail/skip status and duration -- For failed tests: `` (assertion error) and `` -- For inconclusive tests: `` (the assumption that was not met) +Saved to `{project_root}/.uloop/outputs/TestResults/.xml`. What it records, including failed suites: `references/xml-results.md`. diff --git a/.agents/skills/uloop-run-tests/references/xml-results.md b/.agents/skills/uloop-run-tests/references/xml-results.md new file mode 100644 index 000000000..136fd3d6c --- /dev/null +++ b/.agents/skills/uloop-run-tests/references/xml-results.md @@ -0,0 +1,13 @@ +# run-tests XML result file + +When a test or suite fails or a test ends inconclusive, run-tests saves NUnit XML results to `{project_root}/.uloop/outputs/TestResults/.xml` and returns the path in `XmlPath`. A run in which every test passed or was skipped saves no XML, and `XmlPath` is `null`. + +The XML contains per-test-case results including: + +- Test name and full name +- Pass/fail/skip status and duration +- For failed tests: `` (assertion error) and `` +- For inconclusive tests: `` (the assumption that was not met) +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. + +The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/.claude/skills/uloop-run-tests/SKILL.md b/.claude/skills/uloop-run-tests/SKILL.md index 767a50003..c94853488 100644 --- a/.claude/skills/uloop-run-tests/SKILL.md +++ b/.claude/skills/uloop-run-tests/SKILL.md @@ -6,7 +6,7 @@ description: "Run Unity Test Runner and report detailed results. Use for EditMod # uloop run-tests -Execute Unity Test Runner. When tests fail or end inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. +Execute Unity Test Runner. When a test or suite fails or a test ends inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. `uloop run-tests` automatically compiles pending script changes before running tests. Pass `--skip-compile` only while validating active hot-reload patches, because the compile clears those patches; otherwise let the default compile surface errors and run against current scripts. `--skip-compile` skips only the CLI-side compile: Unity still imports script edits saved since the last compile, and that import reloads the domain as soon as the run releases its assembly lock, discarding active patches and ending the request. @@ -46,9 +46,9 @@ exact matches the full test name (Namespace.Class.Method). class runs every test Returns JSON with: -- `Success` (boolean): Whether every test passed or was skipped; `false` when any failed or was inconclusive +- `Success` (boolean): Whether every test passed or was skipped; `false` when a test or suite failed or a test was inconclusive - `Status` (string): Machine-readable execution status such as `Passed`, `Failed`, `Inconclusive`, `NoTestsFound`, or `ExecutionFailed` -- `HasFailures` (boolean): Whether any discovered test failed +- `HasFailures` (boolean): Whether any discovered test or suite failed - `Message` (string): Summary message - `NoTestsFound` (boolean): Whether Unity Test Runner discovered zero matching tests - `NoTestsFoundExplanation` (string): Agent-facing explanation when `NoTestsFound` is true; empty otherwise @@ -58,19 +58,15 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when a test or suite failed or a test was inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. ### XML Result File -When tests fail or end inconclusive, NUnit XML results are automatically saved to `{project_root}/.uloop/outputs/TestResults/.xml`. The XML contains per-test-case results including: - -- Test name and full name -- Pass/fail/skip status and duration -- For failed tests: `` (assertion error) and `` -- For inconclusive tests: `` (the assumption that was not met) +Saved to `{project_root}/.uloop/outputs/TestResults/.xml`. What it records, including failed suites: `references/xml-results.md`. diff --git a/.claude/skills/uloop-run-tests/references/xml-results.md b/.claude/skills/uloop-run-tests/references/xml-results.md new file mode 100644 index 000000000..136fd3d6c --- /dev/null +++ b/.claude/skills/uloop-run-tests/references/xml-results.md @@ -0,0 +1,13 @@ +# run-tests XML result file + +When a test or suite fails or a test ends inconclusive, run-tests saves NUnit XML results to `{project_root}/.uloop/outputs/TestResults/.xml` and returns the path in `XmlPath`. A run in which every test passed or was skipped saves no XML, and `XmlPath` is `null`. + +The XML contains per-test-case results including: + +- Test name and full name +- Pass/fail/skip status and duration +- For failed tests: `` (assertion error) and `` +- For inconclusive tests: `` (the assumption that was not met) +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. + +The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/Assets/Tests/Editor/RunTestsResponseContractTests.cs b/Assets/Tests/Editor/RunTestsResponseContractTests.cs index 774fb372b..ea91d196e 100644 --- a/Assets/Tests/Editor/RunTestsResponseContractTests.cs +++ b/Assets/Tests/Editor/RunTestsResponseContractTests.cs @@ -265,6 +265,114 @@ public void FromResult_WhenResultHasInconclusiveLeaves_CopiesCountAndDetails() Assert.That(empty.InconclusiveTests, Is.Null); } + /// + /// What: FailedSuites is omitted from JSON when no suite failed and serializes each suite's + /// name and message when one did. + /// + [Test] + public void RunTestsResponse_WhenSerialized_OmitsOrIncludesFailedSuites() + { + RunTestsResponse withoutFailedSuites = CreateFailedSuitesResponse(null); + RunTestsResponse withFailedSuites = CreateFailedSuitesResponse(new[] + { + new SerializableTestResult.FailedTestDetail + { + FullName = "Example.Tests.TearDownFixture", + Message = "TearDown : System.InvalidOperationException : teardown failed" + } + }); + + JObject withoutJson = JObject.Parse( + JsonConvert.SerializeObject( + withoutFailedSuites, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings)); + JObject withJson = JObject.Parse( + JsonConvert.SerializeObject( + withFailedSuites, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings)); + JArray failedSuites = (JArray)withJson["FailedSuites"]; + + Assert.That(withoutJson.Property("FailedSuites"), Is.Null); + Assert.That(failedSuites, Is.Not.Null); + Assert.That(failedSuites.Count, Is.EqualTo(1)); + JObject first = (JObject)failedSuites[0]; + Assert.That(first["FullName"]?.Value(), Is.EqualTo("Example.Tests.TearDownFixture")); + Assert.That( + first["Message"]?.Value(), + Is.EqualTo("TearDown : System.InvalidOperationException : teardown failed")); + } + + /// + /// What: the response built from a stored result carries its failed suites, and leaves + /// FailedSuites unset when the result lists none. + /// + [Test] + public void FromResult_WhenResultHasFailedSuites_CopiesThem() + { + SerializableTestResult withFailedSuites = new SerializableTestResult + { + success = false, + status = RunTestsExecutionStatus.Failed, + hasFailures = true, + message = "Test execution completed with status: Failed", + noTestsFoundExplanation = string.Empty, + completedAt = "2026-01-01T00:00:00.0000000Z", + testCount = 1, + passedCount = 1, + failedSuites = new[] + { + new SerializableTestResult.FailedTestDetail + { + FullName = "Example.Tests.TearDownFixture", + Message = "TearDown : System.InvalidOperationException : teardown failed" + } + } + }; + SerializableTestResult withoutFailedSuites = new SerializableTestResult + { + success = true, + status = RunTestsExecutionStatus.Passed, + message = "Test execution completed with status: Passed", + noTestsFoundExplanation = string.Empty, + completedAt = "2026-01-01T00:00:00.0000000Z", + testCount = 1, + passedCount = 1, + failedSuites = new SerializableTestResult.FailedTestDetail[0] + }; + + RunTestsResponse copied = RunTestsResponseFactory.FromResult(withFailedSuites); + RunTestsResponse empty = RunTestsResponseFactory.FromResult(withoutFailedSuites); + + Assert.That(copied.FailedSuites, Is.Not.Null); + Assert.That(copied.FailedSuites.Length, Is.EqualTo(1)); + Assert.That(copied.FailedSuites[0].FullName, Is.EqualTo("Example.Tests.TearDownFixture")); + Assert.That(empty.FailedSuites, Is.Null); + } + + private static RunTestsResponse CreateFailedSuitesResponse( + SerializableTestResult.FailedTestDetail[] failedSuites) + { + return new RunTestsResponse( + success: failedSuites == null, + message: "Test execution completed", + completedAt: "2026-01-01T00:00:00.0000000Z", + testCount: 1, + passedCount: 1, + failedCount: 0, + skippedCount: 0, + inconclusiveCount: 0, + xmlPath: null, + status: failedSuites == null ? RunTestsExecutionStatus.Passed : RunTestsExecutionStatus.Failed, + hasFailures: failedSuites != null, + noTestsFound: false, + noTestsFoundExplanation: string.Empty) + { + FailedSuites = failedSuites + }; + } + /// /// What: an empty Warning is omitted from production JSON so the key cannot reappear unnoticed. /// diff --git a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs index 63014888b..20fa1ccac 100644 --- a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs +++ b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs @@ -32,6 +32,7 @@ public sealed class RunTestsTestFrameworkResultTests private const short MultiByteOpCodePrefix = 0xFE; private const int SwitchCaseCountSize = 4; private const int SwitchCaseSize = 4; + private const string TearDownFailureMessage = "TearDown : System.InvalidOperationException : teardown failed"; [Test] public void SaveTestResultAsXml_WhenSavingResult_UsesCollisionResistantFileName() @@ -157,6 +158,50 @@ public void SaveTestResultAsXml_WhenALeafIsInconclusive_WritesItsMessageAsReason } } + /// + /// What: the saved XML keeps the message of a fixture that failed outside its tests on the + /// fixture's test-suite element, since no test case carries it. + /// + [Test] + public void SaveTestResultAsXml_WhenAFixtureFailsOutsideItsTests_WritesItsMessageOnTheSuite() + { + string filePath = null; + try + { + XmlDocument document = SaveResultAndLoadXml(CreateRunWithFailedTearDownFixture(), out filePath); + XmlNode suiteMessage = document.SelectSingleNode("//test-suite[@name='TearDownFixture']/failure/message"); + + Assert.That(suiteMessage, Is.Not.Null); + Assert.That(suiteMessage.InnerText, Is.EqualTo(TearDownFailureMessage)); + } + finally + { + DeleteIfExists(filePath); + } + } + + /// + /// What: the saved XML marks the whole run Failed when a fixture failed although every test + /// case passed. + /// + [Test] + public void SaveTestResultAsXml_WhenOnlyAFixtureFailed_MarksTheRunFailed() + { + string filePath = null; + try + { + XmlDocument document = SaveResultAndLoadXml(CreateRunWithFailedTearDownFixture(), out filePath); + XmlNode runResult = document.SelectSingleNode("/test-run/@result"); + + Assert.That(runResult, Is.Not.Null); + Assert.That(runResult.Value, Is.EqualTo("Failed")); + } + finally + { + DeleteIfExists(filePath); + } + } + [Test] public void FromTestResult_WhenResultIsNull_ReturnsFailureWithoutCounts() { @@ -499,6 +544,118 @@ public void FromTestResult_WhenFailedRootHasInconclusiveLeafAndNoFailedLeaf_Keep Assert.That(result.inconclusiveTests, Has.Length.EqualTo(1)); } + /// + /// What: a run in which every test passed but a fixture's OneTimeTearDown threw is reported + /// as a failure instead of a pass. + /// + [Test] + public void FromTestResult_WhenFixtureTearDownFailsAndEveryTestPassed_ReportsFailed() + { + SerializableTestResult result = SerializableTestResultConverter.FromTestResult( + CreateRunWithFailedTearDownFixture()); + + Assert.That(result.success, Is.False); + Assert.That(result.status, Is.EqualTo("Failed")); + Assert.That(result.hasFailures, Is.True); + Assert.That(result.failedCount, Is.EqualTo(0)); + } + + /// + /// What: the fixture that failed outside its tests is listed with its message and the source + /// location from its stack trace. + /// + [Test] + public void FromTestResult_WhenFixtureTearDownFails_ListsTheFixtureWithMessageAndLocation() + { + SerializableTestResult result = SerializableTestResultConverter.FromTestResult( + CreateRunWithFailedTearDownFixture()); + + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.TearDownFixture")); + Assert.That(result.failedSuites[0].Message, Is.EqualTo(TearDownFailureMessage)); + Assert.That(result.failedSuites[0].File, Is.EqualTo("Assets/Tests/TearDownFixture.cs")); + Assert.That(result.failedSuites[0].Line, Is.EqualTo(12)); + } + + /// + /// What: when a setup fixture's OneTimeTearDown throws, only that suite is listed, not the + /// ancestors whose Failed status merely rolls it up. + /// + [Test] + public void FromTestResult_WhenSetUpFixtureTearDownFails_ListsOnlyTheSetUpFixture() + { + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "NamespaceSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "SetUpFixtureSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "PassingFixture", + TestResultStatus.Passed, + 0.1, + new List + { + CreateTestCase("PassingTest", TestResultStatus.Passed, 0.1) + }) + }, + TearDownFailureMessage) + }, + "One or more child tests had errors") + }, + "One or more child tests had errors"); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.SetUpFixtureSuite")); + } + + /// + /// What: a fixture that is Failed only because one of its tests failed is not listed as a + /// failed suite; the failed test already carries the failure. + /// + [Test] + public void FromTestResult_WhenATestFails_DoesNotListItsFixtureAsAFailedSuite() + { + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "FixtureWithFailingTest", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestCase("FailingTest", TestResultStatus.Failed, 0.1, "Expected 2 But was: 1") + }, + "One or more child tests had errors") + }, + "One or more child tests had errors"); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedCount, Is.EqualTo(1)); + Assert.That(result.failedSuites, Is.Null); + } + /// /// What: a non-Passed root aggregate containing an inconclusive leaf remains non-successful. /// @@ -708,6 +865,32 @@ public void ShouldSaveResultXml_WhenLeavesOnlyPassedOrWereSkipped_ReturnsFalse() Assert.That(SerializableTestResultConverter.ShouldSaveResultXml(result), Is.False); } + /// + /// What: a run whose only failure is a suite that failed outside its tests still saves the + /// result XML, which keeps the suite's message. + /// + [Test] + public void ShouldSaveResultXml_WhenOnlyASuiteFailed_ReturnsTrue() + { + SerializableTestResult result = new SerializableTestResult + { + testCount = 1, + passedCount = 1, + failedCount = 0, + inconclusiveCount = 0, + failedSuites = new[] + { + new SerializableTestResult.FailedTestDetail + { + FullName = "Example.Tests.TearDownFixture", + Message = TearDownFailureMessage + } + } + }; + + Assert.That(SerializableTestResultConverter.ShouldSaveResultXml(result), Is.True); + } + [Test] public void SaveTestResultAsXml_DoesNotCallAssetDatabaseRefresh() { @@ -871,10 +1054,35 @@ private static ITestResultAdaptor CreateTestSuite( string name, TestResultStatus status, double durationSeconds, - IReadOnlyList children) + IReadOnlyList children, + string message = "", + string stackTrace = "") { FakeTestAdaptor test = new FakeTestAdaptor(name, true); - return new FakeTestResultAdaptor(test, status, durationSeconds, children); + return new FakeTestResultAdaptor(test, status, durationSeconds, children, message, stackTrace); + } + + // Mirrors how Unity reports a fixture whose OneTimeTearDown threw: the fixture is Failed + // with the teardown message while its only test keeps Passed, and the root rolls up Failed. + private static ITestResultAdaptor CreateRunWithFailedTearDownFixture() + { + return CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "TearDownFixture", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestCase("PassingTest", TestResultStatus.Passed, 0.1) + }, + TearDownFailureMessage, + "--TearDown\n at Example.Tests.TearDownFixture.TearDownOnce () [0x00000] in /ignored/path.cs:1\n (at Assets/Tests/TearDownFixture.cs:12)") + }); } private static ITestResultAdaptor CreateTestCase( diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponse.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponse.cs index b10956209..40917099c 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponse.cs @@ -104,6 +104,12 @@ public class RunTestsResponse : UnityCliLoopToolResponse [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public SerializableTestResult.InconclusiveTestDetail[] InconclusiveTests { get; set; } + /// + /// Suites that failed outside their tests, omitted from JSON when none failed. + /// + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public SerializableTestResult.FailedTestDetail[] FailedSuites { get; set; } + /// /// Policy warning when hot-reload changes were live at test-run start. Empty when none /// were active; omitted from JSON via ShouldSerializeWarning. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponseFactory.cs index 4a42a0aaf..5002d4701 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsResponseFactory.cs @@ -57,6 +57,11 @@ private static void CopyTestDetails(SerializableTestResult result, RunTestsRespo { response.InconclusiveTests = result.inconclusiveTests; } + + if (result.failedSuites != null && result.failedSuites.Length > 0) + { + response.FailedSuites = result.failedSuites; + } } } } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md index 767a50003..c94853488 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md @@ -6,7 +6,7 @@ description: "Run Unity Test Runner and report detailed results. Use for EditMod # uloop run-tests -Execute Unity Test Runner. When tests fail or end inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. +Execute Unity Test Runner. When a test or suite fails or a test ends inconclusive, NUnit XML results with failure messages, stack traces, and inconclusive reasons are automatically saved. Read the XML file at `XmlPath` for detailed diagnosis. `uloop run-tests` automatically compiles pending script changes before running tests. Pass `--skip-compile` only while validating active hot-reload patches, because the compile clears those patches; otherwise let the default compile surface errors and run against current scripts. `--skip-compile` skips only the CLI-side compile: Unity still imports script edits saved since the last compile, and that import reloads the domain as soon as the run releases its assembly lock, discarding active patches and ending the request. @@ -46,9 +46,9 @@ exact matches the full test name (Namespace.Class.Method). class runs every test Returns JSON with: -- `Success` (boolean): Whether every test passed or was skipped; `false` when any failed or was inconclusive +- `Success` (boolean): Whether every test passed or was skipped; `false` when a test or suite failed or a test was inconclusive - `Status` (string): Machine-readable execution status such as `Passed`, `Failed`, `Inconclusive`, `NoTestsFound`, or `ExecutionFailed` -- `HasFailures` (boolean): Whether any discovered test failed +- `HasFailures` (boolean): Whether any discovered test or suite failed - `Message` (string): Summary message - `NoTestsFound` (boolean): Whether Unity Test Runner discovered zero matching tests - `NoTestsFoundExplanation` (string): Agent-facing explanation when `NoTestsFound` is true; empty otherwise @@ -58,19 +58,15 @@ Returns JSON with: - `FailedCount` (number): Failed tests - `SkippedCount` (number): Skipped tests - `InconclusiveCount` (number): Inconclusive tests (an `Assume` was not met) -- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when tests failed or were inconclusive and the file exists on disk. +- `XmlPath` (string or null): Path to NUnit XML result file. `null` when no XML was saved (typically on `Success: true`); set only when a test or suite failed or a test was inconclusive and the file exists on disk. - `ClearedPausePointIds` (string[], optional): IDs of pause points that were cleared before test execution. Omitted from JSON when no pause points were active. - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. ### XML Result File -When tests fail or end inconclusive, NUnit XML results are automatically saved to `{project_root}/.uloop/outputs/TestResults/.xml`. The XML contains per-test-case results including: - -- Test name and full name -- Pass/fail/skip status and duration -- For failed tests: `` (assertion error) and `` -- For inconclusive tests: `` (the assumption that was not met) +Saved to `{project_root}/.uloop/outputs/TestResults/.xml`. What it records, including failed suites: `references/xml-results.md`. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references.meta b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references.meta new file mode 100644 index 000000000..104451af4 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 363c3cf38294f4fde88b3f2257a0ede2 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md new file mode 100644 index 000000000..136fd3d6c --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md @@ -0,0 +1,13 @@ +# run-tests XML result file + +When a test or suite fails or a test ends inconclusive, run-tests saves NUnit XML results to `{project_root}/.uloop/outputs/TestResults/.xml` and returns the path in `XmlPath`. A run in which every test passed or was skipped saves no XML, and `XmlPath` is `null`. + +The XML contains per-test-case results including: + +- Test name and full name +- Pass/fail/skip status and duration +- For failed tests: `` (assertion error) and `` +- For inconclusive tests: `` (the assumption that was not met) +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. + +The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md.meta b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md.meta new file mode 100644 index 000000000..247cd09ac --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: ad41812d09cd4435c8899bb747d459ec +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/NUnitXmlResultExporter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/NUnitXmlResultExporter.cs index db386efd3..e4951993f 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/NUnitXmlResultExporter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/NUnitXmlResultExporter.cs @@ -136,6 +136,15 @@ private static XmlElement CreateTestSuiteElement(XmlDocument document, ITestResu suite.SetAttribute("skipped", CountSkipped(result).ToString()); suite.SetAttribute("inconclusive", CountInconclusive(result).ToString()); + // Why on the suite: a OneTimeTearDown error fails the fixture but none of its test cases, + // so the suite is the only element that can carry its message. + if (result.TestStatus == TestStatus.Failed && !string.IsNullOrEmpty(result.Message)) + { + XmlElement failure = document.CreateElement("failure"); + AppendFailureMessage(document, failure, result); + suite.AppendChild(failure); + } + if (result.Children == null) { return suite; @@ -214,7 +223,9 @@ private static void AppendFailureMessage(XmlDocument document, XmlElement failur private static string GetOverallResult(ITestResultAdaptor result) { - if (CountFailed(result) > 0) + // Why the root status too: a fixture that failed outside its tests fails the run with + // no failed test case to count. + if (CountFailed(result) > 0 || result.TestStatus == TestStatus.Failed) { return "Failed"; } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs index 2d24f3e62..3f9c5e087 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs @@ -47,8 +47,9 @@ public static SerializableTestResult FromTestResult(ITestResultAdaptor result) int failedTests = CountFailedTests(result); int skippedTests = CountSkippedTests(result); int inconclusiveTests = CountInconclusiveTests(result); + SerializableTestResult.FailedTestDetail[] failedSuites = CollectFailedSuiteDetails(result); bool noTestsFound = totalTests == 0; - bool hasFailures = failedTests > 0; + bool hasFailures = failedTests > 0 || failedSuites != null; RunTestsResultClassification classification = Classify( result, totalTests, @@ -82,17 +83,20 @@ public static SerializableTestResult FromTestResult(ITestResultAdaptor result) xmlPath = null, failedTests = CollectFailedTestDetails(result), skippedTests = CollectSkippedTestFullNames(result), - inconclusiveTests = CollectInconclusiveTestDetails(result) + inconclusiveTests = CollectInconclusiveTestDetails(result), + failedSuites = failedSuites }; } /// /// Whether a finished run leaves anything to read in the NUnit XML: a failed or an - /// inconclusive leaf. + /// inconclusive leaf, or a suite that failed outside its tests. /// internal static bool ShouldSaveResultXml(SerializableTestResult result) { - return result.failedCount > 0 || result.inconclusiveCount > 0; + return result.failedCount > 0 + || result.inconclusiveCount > 0 + || (result.failedSuites != null && result.failedSuites.Length > 0); } private static RunTestsResultClassification Classify( @@ -289,6 +293,73 @@ private static SerializableTestResult.FailedTestDetail CreateFailedTestDetail(IT }; } + private static SerializableTestResult.FailedTestDetail[] CollectFailedSuiteDetails(ITestResultAdaptor result) + { + List details = + new List(); + AppendFailedSuiteDetails(result, details); + if (details.Count == 0) + { + return null; + } + + return details.ToArray(); + } + + // Why only the deepest Failed suite: NUnit rolls a failure up into every ancestor suite, so + // a Failed suite is where the failure started only when neither a child suite nor one of its + // tests failed. That is a OneTimeTearDown error, which leaves every test's own result alone. + private static void AppendFailedSuiteDetails( + ITestResultAdaptor result, + List details) + { + if (details.Count >= RunTestsConstants.FailedTestDetailsLimit) + { + return; + } + + if (!result.Test.IsSuite || result.TestStatus != TestStatus.Failed) + { + return; + } + + if (AppendFailedChildSuiteDetails(result, details)) + { + return; + } + + if (CountFailedTests(result) > 0) + { + return; + } + + details.Add(CreateFailedTestDetail(result)); + } + + private static bool AppendFailedChildSuiteDetails( + ITestResultAdaptor result, + List details) + { + if (result.Children == null) + { + return false; + } + + bool hasFailedChildSuite = false; + foreach (ITestResultAdaptor child in result.Children) + { + if (!child.Test.IsSuite || child.TestStatus != TestStatus.Failed) + { + continue; + } + + hasFailedChildSuite = true; + AppendFailedSuiteDetails(child, details); + } + + return hasFailedChildSuite; + } + private static string[] CollectSkippedTestFullNames(ITestResultAdaptor result) { List fullNames = new List(); diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestRunner/SerializableTestResult.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestRunner/SerializableTestResult.cs index 56ecba744..445eef796 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestRunner/SerializableTestResult.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestRunner/SerializableTestResult.cs @@ -39,6 +39,12 @@ public class SerializableTestResult /// public InconclusiveTestDetail[] inconclusiveTests; + /// + /// Suites that failed outside their tests, such as a fixture whose OneTimeTearDown threw, + /// capped for the JSON response. Null when none failed. + /// + public FailedTestDetail[] failedSuites; + /// /// One inconclusive test leaf included in a run-tests response, with the message that names /// the assumption it could not meet. From ff23bc9b3fe08ff0e3b9ce1e965b9862d83143a3 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 00:49:26 +0900 Subject: [PATCH 3/3] List a suite whose own one-time setup or teardown failed alongside failed tests The previous rule listed only the deepest Failed suite with no failed child suite and no failed test. It dropped a fixture whose OneTimeTearDown threw while one of its tests also failed, and a setup fixture whose OneTimeTearDown threw above a failing fixture, so an agent could not see the teardown error until the test failure was fixed. - NUnit records a one-time setup or teardown error on the suite's own result state at the (SetUp) or (TearDown) site and rolls failures into ancestors at the (Child) site, so that site now decides. - The old structural rule stays as the fallback for a Failed suite with neither site and nothing Failed beneath it, such as a cancelled one; without it a cancelled run whose executed tests all passed reports success. - A OneTimeSetUp failure now lists the suite too. It is the only entry carrying the throw's file and line, since its tests only get "OneTimeSetUp: ..." messages. The test fake now carries the result state strings Unity's NUnit produces. The skill names OneTimeSetUp in the FailedSuites bullet and no longer says a throwing fixture's tests stay passed; the generated copies are regenerated. --- .agents/skills/uloop-run-tests/SKILL.md | 2 +- .../uloop-run-tests/references/xml-results.md | 2 +- .claude/skills/uloop-run-tests/SKILL.md | 2 +- .../uloop-run-tests/references/xml-results.md | 2 +- .../RunTestsTestFrameworkResultTests.cs | 221 ++++++++++++++++-- .../FirstPartyTools/RunTests/Skill/SKILL.md | 2 +- .../RunTests/Skill/references/xml-results.md | 2 +- .../SerializableTestResultConverter.cs | 55 +++-- 8 files changed, 250 insertions(+), 38 deletions(-) diff --git a/.agents/skills/uloop-run-tests/SKILL.md b/.agents/skills/uloop-run-tests/SKILL.md index c94853488..8e3bdd7a7 100644 --- a/.agents/skills/uloop-run-tests/SKILL.md +++ b/.agents/skills/uloop-run-tests/SKILL.md @@ -63,7 +63,7 @@ Returns JSON with: - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. -- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeSetUp` or `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. diff --git a/.agents/skills/uloop-run-tests/references/xml-results.md b/.agents/skills/uloop-run-tests/references/xml-results.md index 136fd3d6c..bd2c563d2 100644 --- a/.agents/skills/uloop-run-tests/references/xml-results.md +++ b/.agents/skills/uloop-run-tests/references/xml-results.md @@ -8,6 +8,6 @@ The XML contains per-test-case results including: - Pass/fail/skip status and duration - For failed tests: `` (assertion error) and `` - For inconclusive tests: `` (the assumption that was not met) -- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`: its test cases keep their own results, passed or failed. The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/.claude/skills/uloop-run-tests/SKILL.md b/.claude/skills/uloop-run-tests/SKILL.md index c94853488..8e3bdd7a7 100644 --- a/.claude/skills/uloop-run-tests/SKILL.md +++ b/.claude/skills/uloop-run-tests/SKILL.md @@ -63,7 +63,7 @@ Returns JSON with: - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. -- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeSetUp` or `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. diff --git a/.claude/skills/uloop-run-tests/references/xml-results.md b/.claude/skills/uloop-run-tests/references/xml-results.md index 136fd3d6c..bd2c563d2 100644 --- a/.claude/skills/uloop-run-tests/references/xml-results.md +++ b/.claude/skills/uloop-run-tests/references/xml-results.md @@ -8,6 +8,6 @@ The XML contains per-test-case results including: - Pass/fail/skip status and duration - For failed tests: `` (assertion error) and `` - For inconclusive tests: `` (the assumption that was not met) -- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`: its test cases keep their own results, passed or failed. The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs index 20fa1ccac..e0db32750 100644 --- a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs +++ b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs @@ -33,6 +33,16 @@ public sealed class RunTestsTestFrameworkResultTests private const int SwitchCaseCountSize = 4; private const int SwitchCaseSize = 4; private const string TearDownFailureMessage = "TearDown : System.InvalidOperationException : teardown failed"; + private const string SetUpFailureMessage = "System.InvalidOperationException : setup failed"; + private const string ChildFailureMessage = "One or more child tests had errors"; + + // Unity's NUnit renders a suite's result state as Status[:Label][(Site)]; these are the + // strings it produces for the suite outcomes the tests below model. + private const string ChildFailureResultState = "Failed(Child)"; + private const string TearDownErrorResultState = "Failed:Error(TearDown)"; + private const string SetUpErrorResultState = "Failed:Error(SetUp)"; + private const string ParentErrorResultState = "Failed:Error(Parent)"; + private const string CancelledResultState = "Failed:Cancelled"; [Test] public void SaveTestResultAsXml_WhenSavingResult_UsesCollisionResistantFileName() @@ -612,11 +622,14 @@ public void FromTestResult_WhenSetUpFixtureTearDownFails_ListsOnlyTheSetUpFixtur CreateTestCase("PassingTest", TestResultStatus.Passed, 0.1) }) }, - TearDownFailureMessage) + TearDownFailureMessage, + resultState: TearDownErrorResultState) }, - "One or more child tests had errors") + ChildFailureMessage, + resultState: ChildFailureResultState) }, - "One or more child tests had errors"); + ChildFailureMessage, + resultState: ChildFailureResultState); SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); @@ -646,9 +659,11 @@ public void FromTestResult_WhenATestFails_DoesNotListItsFixtureAsAFailedSuite() { CreateTestCase("FailingTest", TestResultStatus.Failed, 0.1, "Expected 2 But was: 1") }, - "One or more child tests had errors") + ChildFailureMessage, + resultState: ChildFailureResultState) }, - "One or more child tests had errors"); + ChildFailureMessage, + resultState: ChildFailureResultState); SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); @@ -656,6 +671,171 @@ public void FromTestResult_WhenATestFails_DoesNotListItsFixtureAsAFailedSuite() Assert.That(result.failedSuites, Is.Null); } + /// + /// What: a fixture whose OneTimeTearDown threw is listed even when one of its tests failed + /// too, so the teardown error is not hidden behind the test failure. + /// + [Test] + public void FromTestResult_WhenFixtureTearDownFailsAlongsideAFailedTest_ListsTheFixture() + { + string fixtureMessage = ChildFailureMessage + "\n" + TearDownFailureMessage; + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "TearDownFixture", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestCase("FailingTest", TestResultStatus.Failed, 0.1, "Expected 2 But was: 1") + }, + fixtureMessage, + resultState: TearDownErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedCount, Is.EqualTo(1)); + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.TearDownFixture")); + Assert.That(result.failedSuites[0].Message, Is.EqualTo(fixtureMessage)); + } + + /// + /// What: a setup fixture whose OneTimeTearDown threw is listed even when a fixture under it + /// failed, while that fixture, Failed only through its failing test, is not. + /// + [Test] + public void FromTestResult_WhenSetUpFixtureTearDownFailsAlongsideAFailedFixture_ListsOnlyTheSetUpFixture() + { + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "SetUpFixtureSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "FixtureWithFailingTest", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestCase("FailingTest", TestResultStatus.Failed, 0.1, "Expected 2 But was: 1") + }, + ChildFailureMessage, + resultState: ChildFailureResultState) + }, + ChildFailureMessage + "\n" + TearDownFailureMessage, + resultState: TearDownErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.SetUpFixtureSuite")); + } + + /// + /// What: when a setup fixture's OneTimeSetUp throws, that suite is listed, while the fixture + /// it skipped, which inherits the failure from it, is not. + /// + [Test] + public void FromTestResult_WhenSetUpFixtureSetUpFails_ListsOnlyTheSetUpFixture() + { + string inheritedMessage = "OneTimeSetUp: " + SetUpFailureMessage; + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "SetUpFixtureSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "FixtureUnderFailedSetUp", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestCase("TestUnderFailedSetUp", TestResultStatus.Failed, 0.1, inheritedMessage) + }, + inheritedMessage, + resultState: ParentErrorResultState) + }, + SetUpFailureMessage, + resultState: SetUpErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedCount, Is.EqualTo(1)); + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.SetUpFixtureSuite")); + } + + /// + /// What: a run cancelled after every executed test passed is reported as a failure that lists + /// only the cancelled fixture, not the ancestors cancelled along with it. + /// + [Test] + public void FromTestResult_WhenAFixtureIsCancelled_ListsOnlyThatFixtureAndReportsFailed() + { + ITestResultAdaptor resultAdaptor = CreateTestSuite( + "RootSuite", + TestResultStatus.Failed, + 0.1, + new List + { + CreateTestSuite( + "PassingFixture", + TestResultStatus.Passed, + 0.1, + new List + { + CreateTestCase("PassingTest", TestResultStatus.Passed, 0.1) + }), + CreateTestSuite( + "CancelledFixture", + TestResultStatus.Failed, + 0.1, + new List(), + "Test cancelled by user", + resultState: CancelledResultState) + }, + "Cancelled by user", + resultState: CancelledResultState); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.success, Is.False); + Assert.That(result.failedSuites, Is.Not.Null); + Assert.That(result.failedSuites.Length, Is.EqualTo(1)); + Assert.That(result.failedSuites[0].FullName, Is.EqualTo("Example.Tests.CancelledFixture")); + } + /// /// What: a non-Passed root aggregate containing an inconclusive leaf remains non-successful. /// @@ -1056,14 +1236,23 @@ private static ITestResultAdaptor CreateTestSuite( double durationSeconds, IReadOnlyList children, string message = "", - string stackTrace = "") + string stackTrace = "", + string resultState = null) { FakeTestAdaptor test = new FakeTestAdaptor(name, true); - return new FakeTestResultAdaptor(test, status, durationSeconds, children, message, stackTrace); + return new FakeTestResultAdaptor( + test, + status, + durationSeconds, + children, + message, + stackTrace, + resultState); } - // Mirrors how Unity reports a fixture whose OneTimeTearDown threw: the fixture is Failed - // with the teardown message while its only test keeps Passed, and the root rolls up Failed. + // Mirrors how Unity reports a fixture whose OneTimeTearDown threw: the fixture is Failed at + // the TearDown site with the teardown message while its only test keeps Passed, and the root + // rolls it up as a child failure. private static ITestResultAdaptor CreateRunWithFailedTearDownFixture() { return CreateTestSuite( @@ -1081,8 +1270,11 @@ private static ITestResultAdaptor CreateRunWithFailedTearDownFixture() CreateTestCase("PassingTest", TestResultStatus.Passed, 0.1) }, TearDownFailureMessage, - "--TearDown\n at Example.Tests.TearDownFixture.TearDownOnce () [0x00000] in /ignored/path.cs:1\n (at Assets/Tests/TearDownFixture.cs:12)") - }); + "--TearDown\n at Example.Tests.TearDownFixture.TearDownOnce () [0x00000] in /ignored/path.cs:1\n (at Assets/Tests/TearDownFixture.cs:12)", + TearDownErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); } private static ITestResultAdaptor CreateTestCase( @@ -1110,6 +1302,7 @@ private sealed class FakeTestResultAdaptor : ITestResultAdaptor private readonly IReadOnlyList _children; private readonly string _message; private readonly string _stackTrace; + private readonly string _resultState; public FakeTestResultAdaptor( ITestAdaptor test, @@ -1117,7 +1310,8 @@ public FakeTestResultAdaptor( double durationSeconds, IReadOnlyList children, string message = "", - string stackTrace = "") + string stackTrace = "", + string resultState = null) { _test = test; _status = status; @@ -1125,12 +1319,13 @@ public FakeTestResultAdaptor( _children = children; _message = message ?? string.Empty; _stackTrace = stackTrace ?? string.Empty; + _resultState = resultState ?? status.ToString(); } public ITestAdaptor Test => _test; public string Name => _test.Name; public string FullName => _test.FullName; - public string ResultState => _status.ToString(); + public string ResultState => _resultState; public TestResultStatus TestStatus => _status; public double Duration => _durationSeconds; public DateTime StartTime => new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md index c94853488..8e3bdd7a7 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/SKILL.md @@ -63,7 +63,7 @@ Returns JSON with: - `FailedTests` (array, optional): Up to 10 failed leaf tests with `FullName`, `Message`, and when the stack trace contains a path:line location, `File` and `Line`. Omitted when no tests failed. When `FailedCount` is greater than 10, `Message` ends with `first 10 of N failures listed; see XmlPath for full results.` - `SkippedTests` (string[], optional): Up to 10 full names of skipped leaf tests. Omitted when no tests were skipped. When `SkippedCount` is greater than 10, only the first 10 names are listed. - `InconclusiveTests` (array, optional): Up to 10 inconclusive leaf tests with `FullName` and `Message`. Omitted when no test was inconclusive. When `InconclusiveCount` is greater than 10, only the first 10 are listed; the XML at `XmlPath` has every message. -- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. +- `FailedSuites` (array, optional): Up to 10 suites that failed outside their tests (e.g. a `OneTimeSetUp` or `OneTimeTearDown` threw), with the `FailedTests` fields. The run is `Failed` even when `FailedCount` is 0. Omitted when none. - `ProposedTestAsmdef` (object, optional): `AssetPath` and `Content` of a ready-to-write test `.asmdef` (test-assembly wiring plus references to the project's assemblies under test). Present only when an unfiltered run found no tests and no test assembly exists for the TestMode. - `CompileNote` (string, optional): States that the automatic compile ran and succeeded before the tests and names `--skip-compile` as the opt-out. When the compile response carried a Warning (for example active hot-reload changes dropped by the domain reload), the note repeats it. Omitted when `--skip-compile` was passed; a failed compile returns the compile error response instead. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md index 136fd3d6c..bd2c563d2 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md +++ b/Packages/src/Editor/FirstPartyTools/RunTests/Skill/references/xml-results.md @@ -8,6 +8,6 @@ The XML contains per-test-case results including: - Pass/fail/skip status and duration - For failed tests: `` (assertion error) and `` - For inconclusive tests: `` (the assumption that was not met) -- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`, because its test cases stay passed. +- For failed suites: `` with `` and `` on the ``. A fixture whose `OneTimeTearDown` threw keeps its error only here and in `FailedSuites`: its test cases keep their own results, passed or failed. The response lists at most 10 entries in each of `FailedTests`, `InconclusiveTests`, and `FailedSuites`; the XML keeps every one. diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs index 3f9c5e087..4eb88b3d2 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/SerializableTestResultConverter.cs @@ -306,9 +306,6 @@ private static SerializableTestResult.FailedTestDetail[] CollectFailedSuiteDetai return details.ToArray(); } - // Why only the deepest Failed suite: NUnit rolls a failure up into every ancestor suite, so - // a Failed suite is where the failure started only when neither a child suite nor one of its - // tests failed. That is a OneTimeTearDown error, which leaves every test's own result alone. private static void AppendFailedSuiteDetails( ITestResultAdaptor result, List details) @@ -323,41 +320,61 @@ private static void AppendFailedSuiteDetails( return; } - if (AppendFailedChildSuiteDetails(result, details)) + if (FailedOutsideItsTests(result)) { - return; + details.Add(CreateFailedTestDetail(result)); } - if (CountFailedTests(result) > 0) + if (result.Children == null) { return; } - details.Add(CreateFailedTestDetail(result)); + foreach (ITestResultAdaptor child in result.Children) + { + AppendFailedSuiteDetails(child, details); + } } - private static bool AppendFailedChildSuiteDetails( - ITestResultAdaptor result, - List details) + // Why two rules: NUnit records a OneTimeSetUp or OneTimeTearDown error on the suite's own + // result state at the SetUp or TearDown site and rolls it into every ancestor at the Child + // site, so that site marks where the failure started even when some tests failed too. A + // Failed suite with neither site and nothing Failed beneath it, such as a cancelled one, + // would otherwise leave a Failed run with nothing that explains it. + private static bool FailedOutsideItsTests(ITestResultAdaptor suite) { - if (result.Children == null) + if (HasSetUpOrTearDownSite(suite.ResultState)) + { + return true; + } + + return !HasFailedChildSuite(suite) && CountFailedTests(suite) == 0; + } + + // Why the text: ITestResultAdaptor exposes the site only through NUnit's ResultState + // string, which renders as Status[:Label][(Site)]. + private static bool HasSetUpOrTearDownSite(string resultState) + { + return resultState.EndsWith("(SetUp)", StringComparison.Ordinal) + || resultState.EndsWith("(TearDown)", StringComparison.Ordinal); + } + + private static bool HasFailedChildSuite(ITestResultAdaptor suite) + { + if (suite.Children == null) { return false; } - bool hasFailedChildSuite = false; - foreach (ITestResultAdaptor child in result.Children) + foreach (ITestResultAdaptor child in suite.Children) { - if (!child.Test.IsSuite || child.TestStatus != TestStatus.Failed) + if (child.Test.IsSuite && child.TestStatus == TestStatus.Failed) { - continue; + return true; } - - hasFailedChildSuite = true; - AppendFailedSuiteDetails(child, details); } - return hasFailedChildSuite; + return false; } private static string[] CollectSkippedTestFullNames(ITestResultAdaptor result)