From dc98b1d9b53227e9e27680663bd648f7e993f146 Mon Sep 17 00:00:00 2001 From: Andrew Skowronski Date: Tue, 15 Sep 2026 15:26:23 -0400 Subject: [PATCH 1/2] [#115] analyze fails when nothing was successfully analyzed The analyze command returned exit code 0 unconditionally once it ran, so a run in which every file was skipped or failed - a Player build without TypeTrees, for example - reported success and left a fully-formed but empty database behind. Both signals a caller can check said the analysis worked. Analyze now returns 1 when no file was processed successfully, explains on stderr why the run produced nothing, and discards the empty database so the output file is not mistaken for a result. Runs that processed at least one file still return 0, even when other files in the input failed. SQLiteWriter disables connection pooling so that disposing it really releases the file, and owns the discard. --- Analyzer/AnalyzerTool.cs | 24 ++++- Analyzer/SQLite/Writers/SQLiteWriter.cs | 9 ++ .../AnalyzeContentLayoutTests.cs | 14 +-- UnityDataTool.Tests/AnalyzeExitCodeTests.cs | 97 +++++++++++++++++++ .../UnityDataToolAssetBundleTests.cs | 14 +-- .../UnityDataToolPlayerDataTests.cs | 12 ++- 6 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 UnityDataTool.Tests/AnalyzeExitCodeTests.cs diff --git a/Analyzer/AnalyzerTool.cs b/Analyzer/AnalyzerTool.cs index a3c7aed..ff7fe9e 100644 --- a/Analyzer/AnalyzerTool.cs +++ b/Analyzer/AnalyzerTool.cs @@ -186,7 +186,29 @@ public int Analyze(AnalyzeOptions options) Console.WriteLine(); Console.WriteLine($"Total time: {(timer.Elapsed.TotalMilliseconds / 1000.0):F3} s"); - return 0; + if (countSuccess > 0) + { + return 0; + } + + // An empty database left behind lets a caller that only checks for the output file mistake + // this run for a success. + Console.Error.WriteLine("Error: no files were successfully analyzed. Discarding the empty database."); + if (countNoTypeTrees > 0) + { + Console.Error.WriteLine($"{countNoTypeTrees} SerializedFiles were skipped because they have no TypeTrees."); + } + + try + { + writer.Discard(); + } + catch (Exception e) + { + Console.Error.WriteLine($"Warning: could not delete \"{m_Options.DatabaseName}\": {e.Message}"); + } + + return 1; } // Validates the ContentDirectory-related inputs and prepares the file list (issue #99): diff --git a/Analyzer/SQLite/Writers/SQLiteWriter.cs b/Analyzer/SQLite/Writers/SQLiteWriter.cs index e3b9d99..bc8e5da 100644 --- a/Analyzer/SQLite/Writers/SQLiteWriter.cs +++ b/Analyzer/SQLite/Writers/SQLiteWriter.cs @@ -29,6 +29,8 @@ public void Begin() SqliteConnectionStringBuilder builder = new(); builder.DataSource = m_DatabaseName; builder.Mode = SqliteOpenMode.ReadWriteCreate; + // A pooled connection keeps the file open after Dispose, which blocks deleting it. + builder.Pooling = false; m_Database = new SqliteConnection(builder.ConnectionString); File.WriteAllBytes(m_DatabaseName, Array.Empty()); try @@ -58,6 +60,13 @@ public void End() finalizeCommand.ExecuteNonQuery(); } + // Closes the database and deletes its file, for a run whose result is not worth keeping. + public void Discard() + { + Dispose(); + File.Delete(m_DatabaseName); + } + public void Dispose() { m_Database?.Dispose(); diff --git a/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs b/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs index 31315d4..55c8260 100644 --- a/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs +++ b/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs @@ -18,6 +18,7 @@ public class AnalyzeContentLayoutTests { private string m_TestOutputFolder; private string m_ContentLayoutPath; + private string m_AssetBundlePath; [OneTimeSetUp] public void OneTimeSetup() @@ -25,6 +26,8 @@ public void OneTimeSetup() m_TestOutputFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, "content_layout_test_folder"); m_ContentLayoutPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "Data", "LeadingEdgeBuilds", "BuildReport-ContentDirectory", "ContentLayout.json"); + m_AssetBundlePath = Path.Combine(TestContext.CurrentContext.TestDirectory, + "Data", "LeadingEdgeBuilds", "AssetBundles", "assetbundleroot"); Directory.CreateDirectory(m_TestOutputFolder); Directory.SetCurrentDirectory(m_TestOutputFolder); } @@ -406,11 +409,9 @@ public async Task FindRefs_ContentDirectoryWithLayout_WalksCrossFileChain() [Test] public async Task Analyze_WithoutContentLayout_DoesNotCreateLayoutTables() { - var bundlePath = Path.Combine(TestContext.CurrentContext.TestDirectory, - "Data", "LeadingEdgeBuilds", "AssetBundles", "assetbundleroot"); var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", bundlePath, "-o", databasePath })); + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", m_AssetBundlePath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db, @@ -426,8 +427,9 @@ public async Task Analyze_UnsupportedLayoutVersion_ImportsNothing() File.WriteAllText(Path.Combine(layoutFolder, "ContentLayout.json"), "{\"Version\": 99}"); var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - // Analyze reports the file as failed but the run itself still completes. - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, "-o", databasePath })); + // Analyze reports the file as failed but the run itself still completes. The bundle gives + // the run something to analyze, so the database is kept and can be inspected (issue #115). + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_AssetBundlePath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db, @@ -443,7 +445,7 @@ public async Task Analyze_ContentLayoutWithoutContent_ImportsNothing() File.WriteAllText(Path.Combine(layoutFolder, "ContentLayout.json"), "null"); var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, "-o", databasePath })); + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_AssetBundlePath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db, diff --git a/UnityDataTool.Tests/AnalyzeExitCodeTests.cs b/UnityDataTool.Tests/AnalyzeExitCodeTests.cs new file mode 100644 index 0000000..b5b3b7b --- /dev/null +++ b/UnityDataTool.Tests/AnalyzeExitCodeTests.cs @@ -0,0 +1,97 @@ +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Data.Sqlite; +using NUnit.Framework; + +namespace UnityDataTools.UnityDataTool.Tests; + +#pragma warning disable NUnit2005, NUnit2006 + +// A run that analyzed nothing must fail through both signals a caller can look at: a non-zero exit +// code, and the absence of the output database (issue #115). A run that analyzed something still +// succeeds, even when other files in the same input failed. +public class AnalyzeExitCodeTests +{ + private const string NothingAnalyzedMessage = "no files were successfully analyzed"; + + private string m_TestOutputFolder; + private string m_AssetBundlePath; + private string m_NoTypeTreeBundle; + + [OneTimeSetUp] + public void OneTimeSetup() + { + m_TestOutputFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, "exitcode_test_folder"); + m_AssetBundlePath = Path.Combine(TestContext.CurrentContext.TestDirectory, + "Data", "AssetBundles", "2019.4.0f1", "scenes"); + m_NoTypeTreeBundle = Path.Combine(TestContext.CurrentContext.TestDirectory, + "Data", "AssetBundleTypeTreeVariations", "AssetBundle-NoTypeTree", "small.bundle"); + Directory.CreateDirectory(m_TestOutputFolder); + Directory.SetCurrentDirectory(m_TestOutputFolder); + } + + [TearDown] + public void Teardown() + { + SqliteConnection.ClearAllPools(); + var testDir = new DirectoryInfo(m_TestOutputFolder); + testDir.EnumerateFiles().ToList().ForEach(f => f.Delete()); + testDir.EnumerateDirectories().ToList().ForEach(d => d.Delete(true)); + } + + private static async Task<(int exitCode, string output)> RunAnalyze(params string[] args) + { + var originalOut = System.Console.Out; + var originalError = System.Console.Error; + using var swOut = new StringWriter(); + using var swErr = new StringWriter(); + try + { + System.Console.SetOut(swOut); + System.Console.SetError(swErr); + var exitCode = await Program.Main(new[] { "analyze" }.Concat(args).ToArray()); + return (exitCode, swOut.ToString() + swErr.ToString()); + } + finally + { + System.Console.SetOut(originalOut); + System.Console.SetError(originalError); + } + } + + [Test] + public async Task Analyze_NothingAnalyzed_RemovesDatabaseOfPreviousRun() + { + var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); + + var (firstExitCode, _) = await RunAnalyze(m_AssetBundlePath, "-o", databasePath); + Assert.AreEqual(0, firstExitCode); + Assert.That(File.Exists(databasePath), Is.True); + + var (exitCode, output) = await RunAnalyze(m_NoTypeTreeBundle, "-o", databasePath); + + Assert.AreEqual(1, exitCode); + StringAssert.Contains(NothingAnalyzedMessage, output); + Assert.That(File.Exists(databasePath), Is.False, "Expected the overwritten database to be removed, not left empty"); + } + + [Test] + public async Task Analyze_SomeFilesFailed_SucceedsAndKeepsDatabase() + { + var inputFolder = Path.Combine(m_TestOutputFolder, "mixed_input"); + Directory.CreateDirectory(inputFolder); + File.Copy(m_AssetBundlePath, Path.Combine(inputFolder, Path.GetFileName(m_AssetBundlePath))); + File.Copy(m_NoTypeTreeBundle, Path.Combine(inputFolder, Path.GetFileName(m_NoTypeTreeBundle))); + + var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); + + var (exitCode, output) = await RunAnalyze(inputFolder, "-o", databasePath); + + Assert.AreEqual(0, exitCode); + StringAssert.Contains("Successfully processed files: 1", output); + StringAssert.Contains("Files without TypeTrees: 1", output); + StringAssert.DoesNotContain(NothingAnalyzedMessage, output); + Assert.That(File.Exists(databasePath), Is.True); + } +} diff --git a/UnityDataTool.Tests/UnityDataToolAssetBundleTests.cs b/UnityDataTool.Tests/UnityDataToolAssetBundleTests.cs index a353aab..e4782da 100644 --- a/UnityDataTool.Tests/UnityDataToolAssetBundleTests.cs +++ b/UnityDataTool.Tests/UnityDataToolAssetBundleTests.cs @@ -277,22 +277,16 @@ public async Task Analyze_WithPattern_DatabaseCorrect( } [Test] - public async Task Analyze_WithPatternNoMatch_DatabaseEmpty( + public async Task Analyze_WithPatternNoMatch_FailsWithoutDatabase( [Values("-p *.x", "--search-pattern *.x")] string options) { var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); var analyzePath = Path.Combine(Context.UnityDataFolder); - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", analyzePath }.Concat(options.Split(" ")).ToArray())); - - using var db = SQLTestHelper.OpenDatabase(databasePath); + // A pattern matching no file analyzes nothing, which is a failed run (issue #115). + Assert.AreEqual(1, await Program.Main(new string[] { "analyze", analyzePath }.Concat(options.Split(" ")).ToArray())); - using (var cmd = db.CreateCommand()) - { - cmd.CommandText = "SELECT COUNT(*) FROM objects"; - - Assert.AreEqual(0, cmd.ExecuteScalar()); - } + Assert.That(File.Exists(databasePath), Is.False); } [Test] diff --git a/UnityDataTool.Tests/UnityDataToolPlayerDataTests.cs b/UnityDataTool.Tests/UnityDataToolPlayerDataTests.cs index d399d32..20c69d3 100644 --- a/UnityDataTool.Tests/UnityDataToolPlayerDataTests.cs +++ b/UnityDataTool.Tests/UnityDataToolPlayerDataTests.cs @@ -175,8 +175,9 @@ public async Task Analyze_PlayerDataNoTypeTree_ReportsFailureCorrectly() Console.SetOut(swOut); Console.SetError(swErr); - // Analyze should return 0 even if files fail (non-zero would be a critical error) - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", Path.Combine(testDataFolder, "level0") })); + // Nothing could be analyzed, so the run fails (issue #115). A run with both successes + // and failures still returns 0. + Assert.AreEqual(1, await Program.Main(new string[] { "analyze", Path.Combine(testDataFolder, "level0") })); var output = swOut.ToString() + swErr.ToString(); @@ -186,6 +187,8 @@ public async Task Analyze_PlayerDataNoTypeTree_ReportsFailureCorrectly() // Check that the summary line categorizes the file as missing TypeTrees, not a success. Assert.That(output, Does.Contain("Files without TypeTrees: 1"), "Expected 'Files without TypeTrees: 1' in summary"); Assert.That(output, Does.Contain("Successfully processed files: 0"), "Expected 'Successfully processed files: 0' in summary"); + Assert.That(output, Does.Contain("no files were successfully analyzed"), "Expected an explicit error message"); + Assert.That(File.Exists(Path.Combine(m_TestOutputFolder, "database.db")), Is.False, "Expected the empty database to be deleted"); } finally { @@ -212,14 +215,15 @@ public async Task Analyze_AssetBundleNoTypeTree_ReportsMissingTypeTreesWithoutCr Console.SetOut(swOut); Console.SetError(swErr); - // Analyze should return 0 even when a bundle has no TypeTrees (no crash, no critical error). - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", bundlePath })); + // The bundle is skipped cleanly, which leaves nothing analyzed: the run fails (issue #115). + Assert.AreEqual(1, await Program.Main(new string[] { "analyze", bundlePath })); var output = swOut.ToString() + swErr.ToString(); Assert.That(output, Does.Contain("Skipped (no TypeTrees)"), "Expected the file to be reported as skipped"); Assert.That(output, Does.Contain("Files without TypeTrees: 1"), "Expected 'Files without TypeTrees: 1' in summary"); Assert.That(output, Does.Contain("Successfully processed files: 0"), "Expected 'Successfully processed files: 0' in summary"); + Assert.That(output, Does.Contain("no TypeTrees"), "Expected the summary error to name the missing TypeTrees"); } finally { From 03b2fb379e342a77417738b267fb0f752aecb187 Mon Sep 17 00:00:00 2001 From: Andrew Skowronski Date: Tue, 15 Sep 2026 15:43:27 -0400 Subject: [PATCH 2/2] [#115] Use a ContentDirectory build report in the ContentLayout tests ContentLayout.json only applies to ContentDirectory builds, so pairing it with an AssetBundle was misleading. The tests that need a second, valid input now use the build report of the same ContentDirectory build. Also moves a comment next to the call it explains. --- Analyzer/AnalyzerTool.cs | 4 ++-- UnityDataTool.Tests/AnalyzeContentLayoutTests.cs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Analyzer/AnalyzerTool.cs b/Analyzer/AnalyzerTool.cs index ff7fe9e..72054af 100644 --- a/Analyzer/AnalyzerTool.cs +++ b/Analyzer/AnalyzerTool.cs @@ -191,8 +191,6 @@ public int Analyze(AnalyzeOptions options) return 0; } - // An empty database left behind lets a caller that only checks for the output file mistake - // this run for a success. Console.Error.WriteLine("Error: no files were successfully analyzed. Discarding the empty database."); if (countNoTypeTrees > 0) { @@ -201,6 +199,8 @@ public int Analyze(AnalyzeOptions options) try { + // An empty database left behind lets a caller that only checks for the output file + // mistake this run for a success. writer.Discard(); } catch (Exception e) diff --git a/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs b/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs index 55c8260..0ea6b1c 100644 --- a/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs +++ b/UnityDataTool.Tests/AnalyzeContentLayoutTests.cs @@ -18,7 +18,7 @@ public class AnalyzeContentLayoutTests { private string m_TestOutputFolder; private string m_ContentLayoutPath; - private string m_AssetBundlePath; + private string m_BuildReportPath; [OneTimeSetUp] public void OneTimeSetup() @@ -26,8 +26,8 @@ public void OneTimeSetup() m_TestOutputFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, "content_layout_test_folder"); m_ContentLayoutPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "Data", "LeadingEdgeBuilds", "BuildReport-ContentDirectory", "ContentLayout.json"); - m_AssetBundlePath = Path.Combine(TestContext.CurrentContext.TestDirectory, - "Data", "LeadingEdgeBuilds", "AssetBundles", "assetbundleroot"); + m_BuildReportPath = Path.Combine(TestContext.CurrentContext.TestDirectory, + "Data", "LeadingEdgeBuilds", "BuildReport-ContentDirectory", "f64157fb08bb9f645971d39c1203bd03.buildreport"); Directory.CreateDirectory(m_TestOutputFolder); Directory.SetCurrentDirectory(m_TestOutputFolder); } @@ -411,7 +411,7 @@ public async Task Analyze_WithoutContentLayout_DoesNotCreateLayoutTables() { var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", m_AssetBundlePath, "-o", databasePath })); + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", m_BuildReportPath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db, @@ -427,9 +427,9 @@ public async Task Analyze_UnsupportedLayoutVersion_ImportsNothing() File.WriteAllText(Path.Combine(layoutFolder, "ContentLayout.json"), "{\"Version\": 99}"); var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - // Analyze reports the file as failed but the run itself still completes. The bundle gives - // the run something to analyze, so the database is kept and can be inspected (issue #115). - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_AssetBundlePath, "-o", databasePath })); + // Analyze reports the layout as failed but the run itself still completes. The build report + // gives it something to analyze, so the database is kept and can be inspected (issue #115). + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_BuildReportPath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db, @@ -445,7 +445,7 @@ public async Task Analyze_ContentLayoutWithoutContent_ImportsNothing() File.WriteAllText(Path.Combine(layoutFolder, "ContentLayout.json"), "null"); var databasePath = SQLTestHelper.GetDatabasePath(m_TestOutputFolder); - Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_AssetBundlePath, "-o", databasePath })); + Assert.AreEqual(0, await Program.Main(new string[] { "analyze", layoutFolder, m_BuildReportPath, "-o", databasePath })); using var db = SQLTestHelper.OpenDatabase(databasePath); SQLTestHelper.AssertQueryInt(db,