diff --git a/.agents/skills/uloop-run-tests/SKILL.md b/.agents/skills/uloop-run-tests/SKILL.md index a35cd0179..8e3bdd7a7 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): 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 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 `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. ### 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..bd2c563d2 --- /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`: 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 a35cd0179..8e3bdd7a7 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): 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 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 `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. ### 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..bd2c563d2 --- /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`: 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/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..e0db32750 100644 --- a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs +++ b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs @@ -32,6 +32,17 @@ 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"; + 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() @@ -157,6 +168,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 +554,288 @@ 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, + resultState: TearDownErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState) + }, + 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: 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") + }, + ChildFailureMessage, + resultState: ChildFailureResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); + + SerializableTestResult result = SerializableTestResultConverter.FromTestResult(resultAdaptor); + + Assert.That(result.failedCount, Is.EqualTo(1)); + 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. /// @@ -708,6 +1045,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 +1234,47 @@ private static ITestResultAdaptor CreateTestSuite( string name, TestResultStatus status, double durationSeconds, - IReadOnlyList children) + IReadOnlyList children, + string message = "", + string stackTrace = "", + string resultState = null) { FakeTestAdaptor test = new FakeTestAdaptor(name, true); - return new FakeTestResultAdaptor(test, status, durationSeconds, children); + return new FakeTestResultAdaptor( + test, + status, + durationSeconds, + children, + message, + stackTrace, + resultState); + } + + // 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( + "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)", + TearDownErrorResultState) + }, + ChildFailureMessage, + resultState: ChildFailureResultState); } private static ITestResultAdaptor CreateTestCase( @@ -902,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, @@ -909,7 +1310,8 @@ public FakeTestResultAdaptor( double durationSeconds, IReadOnlyList children, string message = "", - string stackTrace = "") + string stackTrace = "", + string resultState = null) { _test = test; _status = status; @@ -917,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/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 a35cd0179..8e3bdd7a7 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): 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 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 `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. ### 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..bd2c563d2 --- /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`: 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/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..4eb88b3d2 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,90 @@ 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(); + } + + private static void AppendFailedSuiteDetails( + ITestResultAdaptor result, + List details) + { + if (details.Count >= RunTestsConstants.FailedTestDetailsLimit) + { + return; + } + + if (!result.Test.IsSuite || result.TestStatus != TestStatus.Failed) + { + return; + } + + if (FailedOutsideItsTests(result)) + { + details.Add(CreateFailedTestDetail(result)); + } + + if (result.Children == null) + { + return; + } + + foreach (ITestResultAdaptor child in result.Children) + { + AppendFailedSuiteDetails(child, 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 (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; + } + + foreach (ITestResultAdaptor child in suite.Children) + { + if (child.Test.IsSuite && child.TestStatus == TestStatus.Failed) + { + return true; + } + } + + return false; + } + 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.