fix: resolve broker test's source path through PackageInfo instead of hardcoded Assets/unity-mcp#31
Open
dehuaichendragonplus wants to merge 1 commit into
Conversation
… hardcoded Assets/unity-mcp 8 of the 61 EditMode tests in MCPBrokerTransportTests assumed this repo's own Assets/unity-mcp dev-checkout layout and failed in any project that installs the package as a real UPM package (embedded/git/registry), where the source lives under Packages/<name> or Library/PackageCache/<name>@Version instead. Verified in a real consumer project (embedded package): the old hardcoded path resolved to a nonexistent file, the new PackageInfo.FindForAssembly-based resolution finds the real file on disk and via AssetDatabase.
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
1 participant
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.
What changed
MCPBrokerTransportTestshardcoded the broker source path asAssets/unity-mcp/Editor/MCP/Server/Broker/keepalive-broker.cs.txt, which only exists when this repo's own source is checked out directly underAssets/. They always failed in any project that installs the package as a real UPM package (embedded, git, or registry), where the source lives underPackages/<name>orLibrary/PackageCache/<name>@versioninstead.ResolveBrokerSourcePath, used by 7 tests viaCreateBrokerPaths) and theAssetDatabaselookup (BrokerSource_IsVisibleToAssetDatabaseForUnityPackageExport) now resolve throughPackageInfo.FindForAssemblyfirst, falling back to the oldAssets/unity-mcppath only when the assembly isn't part of a package (preserving this repo's own dev-checkout layout).Checklist
Packages/com.gamebooom.unity.mcp)Funplay > MCP Serveropens and starts correctly (N/A to this change).idea/or.DS_StoreCHANGELOG.mdwhen the change affects usersVerification note
Unity's own async Test Runner job tracking (
run_tests/get_test_job, exposed via the Funplay MCP tools this repo builds) got stuck mid-run on EditMode tests in this large consumer project regardless of which tests were selected -- possibly an unrelated pre-existing issue with the job-tracking wrapper in bigger projects, worth a separate look. Verified this fix directly instead: reproducedResolveBrokerSourcePath's and the AssetDatabase lookup's exact logic against the real embedded-package layout and confirmed both resolve correctly, where the old hardcoded path resolved to a nonexistent file.