diff --git a/Analyzer/AnalyzerTool.cs b/Analyzer/AnalyzerTool.cs index a3c7aed..72054af 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; + } + + 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 + { + // 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) + { + 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..0ea6b1c 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_BuildReportPath; [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_BuildReportPath = Path.Combine(TestContext.CurrentContext.TestDirectory, + "Data", "LeadingEdgeBuilds", "BuildReport-ContentDirectory", "f64157fb08bb9f645971d39c1203bd03.buildreport"); 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_BuildReportPath, "-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 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, @@ -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_BuildReportPath, "-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 {