{{ message }}
[#72] Add find-refs test coverage and fix its database/null-ref bugs#79
Merged
Conversation
Rename UnityFileSystemTestData and introduce a LeadingEdge project for testing Content Directory builds and other newer features that require Unity >=6.6. Add AGENTS.md explaining the relationship between these projects and the referenced files checked in to TestCommon
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the find-refs CLI command to work against current analyze-produced databases (issue #72 and a ScriptableObject null-read crash), adds --stdout output support, and introduces deterministic Unity build outputs plus NUnit coverage to prevent regressions.
Changes:
- Fix
find-refsdatabase opening and NULL handling in correlated subqueries; add--stdoutoutput mode and CLI validation. - Add comprehensive
find-refsregression/behavior tests (plus one newdumptest) using checked-in LeadingEdge AssetBundle build output. - Introduce/rename Unity reference projects (
Baseline,LeadingEdge) and check in corresponding reference build/test-data + documentation.
Reviewed changes
Copilot reviewed 74 out of 133 changed files in this pull request and generated 4 comments.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+59
…rage find-refs had no automated tests and two bugs that broke it against any current analyze database: - Issue #72: the database was opened with a hand-written connection string using the legacy System.Data.SQLite "Version=3" keyword, which Microsoft.Data.Sqlite rejects. Build the connection string with SqliteConnectionStringBuilder instead (matching SQLiteWriter). Fixed the same broken string in ExpectedDataGenerator. - A ScriptableObject is a MonoBehaviour whose m_GameObject PPtr is 0, so its game_object column is a non-null id matching no row; the game_object/script subqueries then return NULL and GetString threw. Null-check those reads. Also add a --stdout option to find-refs (mutually exclusive with -o), mirroring dump, and update command-find-refs.md. Add FindRefsTests covering both bugs, name/type lookup and disambiguation, object-id lookup, --find-all, missing objects, empty refs table, the --stdout/-o validator, and direct refs-table queries. Tests run against the LeadingEdge AssetBundle reference build.
SkowronskiAndrew
force-pushed
the
fix-find-ref
branch
from
7c7bdf0 to
e3046f7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch introduces a second Unity reference project used to generate deterministic test data, adds the first tests that run against that build output, and fixes the broken
find-refscommand (the original motivation, issue #72) while adding thorough test coverage for it.Changes
Fix
find-refs(issue #72 + a second bug)find-refshad no automated tests and two bugs that broke it against any database produced by the currentanalyze:System.Data.SQLiteVersion=3keyword, whichMicrosoft.Data.Sqliterejects (Connection string keyword 'version' is not supported). It is now built withSqliteConnectionStringBuilder, matchingSQLiteWriter. The same broken string inExpectedDataGeneratorwas fixed too.m_GameObjectPPtr is0, so itsgame_objectcolumn holds a non-null id that matches no row; thegame_object/scriptcorrelated subqueries then returnNULLandGetStringthrewInvalidOperationException. Those reads are now null-checked. This brokefind-refsfor essentially any ScriptableObject reference chain.--stdoutforfind-refsAdded a
--stdoutoption (mutually exclusive with-o/--output-file), mirroringdump, so chains can be written to the console.command-find-refs.mddocuments the new option and clarifies the traversal behavior:find-refswalks up the reference graph and stops at the first asset it reaches, so chains end at the immediate containing asset rather than the ultimate root.New
find-refstestsUnityDataTool.Tests/FindRefsTests.csrunsanalyzeon the LeadingEdge AssetBundle build in setup and covers:[Script = ...]without crashing (regression for the second bug).AssetBundleRootfrom a leaf asset, and confirming a target search stops at the leaf asset (documents the stop-at-first-asset traversal).--find-all(produces identical chains to the default for this data), non-existent object, empty refs table (--skip-references), and the--stdout/-ovalidator.refs/object_viewqueries validating the underlying reference counts.Second reference Unity project (
LeadingEdge) and reference build dataBaseline(stable LTS versions) and addedLeadingEdge, which tracks the newest Unity version so newer build features (Content Directory builds, serializedDictionary<,>, Loadable`) can be tested. Editor scripts generate ScriptableObjects with well-known relationships between assets and AudioClips and run both AssetBundle and Content Directory builds.TestCommon/Data/LeadingEdgeBuilds(AssetBundles, Content Directory, and build reports), used by the new tests.AGENTS.md/CLAUDE.mddocumentation for the projects and test-data folders.First test using the new build output (
dump)DumpTestsgained a test that dumps theSerializationDemoScriptableObject from the LeadingEdge AssetBundle build and confirms the serialized field layout and values across the core numeric types, the string, the[SerializeReference]managed reference, and the int array.Testing
dotnet test UnityDataTool.Tests— 231 passing, 0 failing.