Repository navigation
fix: failing player tests in netcode.gameobjects (UUM-160259) - #4246
NoelStephensUnity wants to merge 3 commits into
Conversation
Guard the DEBUG-only message size check and UnityTransport warning in their tests, and add a regression test that ownership changes keep every instance registered with ObjectNameIdentifier.
There was a problem hiding this comment.
💡 Harness Review
The test guards match the runtime’s DEBUG-only checks, and the new test covers registration across ownership transfer and removal. I examined the matching runtime branches, registration callbacks, wait behavior, and test teardown.
Reviewed commit f75ee37
🤖 Helpful? 👍/👎
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-3.x.x #4246 +/- ##
=================================================
- Coverage 76.73% 76.72% -0.02%
=================================================
Files 159 159
Lines 28101 28101
=================================================
- Hits 21564 21560 -4
- Misses 6537 6541 +4
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes
|
EmandM
left a comment
There was a problem hiding this comment.
Looks good other than the tests for the test helper
There was a problem hiding this comment.
ObjectNameIdentifier is a test helper. We don't want to add tests for regressions in the test helpers. The tests being helped by the test helpers should be the regression test for those changes.
Purpose of this PR
Fixes the NGO runtime tests that fail in non-development player builds, and adds a regression test for
ObjectNameIdentifierregistration across ownership changes. This replicates trunk PR unity/unity#137865 (shadow package), so the two copies stay in sync.PR Scope:
NamedMessageTests/UnnamedMessageTests: the too-big-buffer tests are ignored whenDEBUGis not defined (the message size check only exists underDEBUG).UnityTransportTests.DoesNotActAfterShutdown: the "should only be called on a listening server" warning is only expected underDEBUG(UnityTransport.DisconnectRemoteClientonly logs it underDEBUG).ObjectNameIdentifierTests.OwnershipChangesKeepAllInstancesRegistered: an object stays registered on every instance after ownership is given away and then removed.Out of scope:
ObjectNameIdentifierchange in the trunk PR (m_CurrentOwner→ the local client id) is already ondevelop-3.x.xasm_RegisteredLocalClientIdvia fix: Yamato jobs not testing built player #4194, so it is not repeated here.Jira ticket
UUM-160259
Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Automated tests:
Covered by new automated testsValidated in trunk on the shadow package PR.
Up-port
N/A (develop-3.x.x specific)
Backports
None. Tests only.