chore: Add a DA service unit test#3423
Merged
EmandM merged 35 commits intodevelop-2.0.0from May 1, 2025
Merged
Conversation
…/com.unity.netcode.gameobjects into chore/rust-unit-test
NoelStephensUnity
approved these changes
Apr 30, 2025
Member
NoelStephensUnity
left a comment
NoelStephensUnity
left a comment
There was a problem hiding this comment.
Only 1 or 2 questions and a minor suggestion on wording for API documentation.
Otherwise, very nice job:
- Cleaning up various areas within the NetcodeIntegrationTest.
- Improved the "readability" in many places.
- Added a more standard way to obtain various NetworkManager instances.
- Cleaning up various integration tests
- Improved the "readability" in many places.
- Created a more uniform standard "flow" and naming.
- Adding UseCMBService to tests that still needed conversion along with comments.
- Removing m_EnableVerboseDebug to reduce over-all test runner log size.
🥇 💯
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Show resolved
Hide resolved
....netcode.gameobjects/Tests/Runtime/DistributedAuthority/NetworkClientAndPlayerObjectTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableCollectionsTests.cs
Show resolved
Hide resolved
testproject/Assets/Tests/Runtime/NetworkSceneManager/InScenePlacedNetworkObjectTests.cs
Show resolved
Hide resolved
This is an attempt to fix an issue where Mac OSX is frequently failing the InterpolationStopAndStartMotionTest. Based on the error, I am thinking (hard to tell since it doesn't replicate locally in-editor nor stand alone) that it could be due to a Mac VM running slower than expected (randomly) which could cause the network tick event to trigger more than once in a single frame.
michalChrobot
approved these changes
May 1, 2025
| # Set this to ensure the DA codec tests will fail if they cannot connect to the echo-server | ||
| # The default is to ignore the codec tests if the echo-server fails to connect | ||
| ENSURE_CODEC_TESTS: "true" | ||
| USE_CMB_SERVICE: "true" |
Member
There was a problem hiding this comment.
I don't see where we are using it 🤔
Member
Author
There was a problem hiding this comment.
It's inside the UseCMBService() function inside NetcodeIntegrationTest.cs (here)
NoelStephensUnity
added a commit
that referenced
this pull request
May 13, 2025
## This PR Includes ### Initial setup changes - Minor improvements to the initial CMB server detection and adjustments to the initial configuration order of operations. - Session owner now starts and connects ahead of the other clients. ### Ignoring tests This PR includes updates that will ignore any test that: - Uses a client-server network topology - _No point in re-running these as they will have already run multiple times on multiple platforms before testing against the CMB server._ - Has overridden UseCmbService and is returning false. - _The test has yet to be converted and so no point in even attempting to run it._ - _This also includes the `TODO: [CmbServiceTests]` to help track all tests that need to be converted or reviewed to determine if they need to be converted._ - Is not derived from NetcodeIntegrationTest. - _Many of these (not all) don't need to run again, but they all have the `TODO: [CmbServiceTests]` to help track all tests that need to be converted or not._ - Does not need to be tested against the CMB Server (i.e. a test for some kind of functionality without actually starting a session and the like). _(This helps to reduce the time to complete the CMB Server integration tests)_ ### Marking/Tracking "for review" tests Assuring that every test that needs to be reviewed includes the `TODO: [CmbServiceTests]` using of the two ways: For classes that derive from `NetcodeIntegrationTest` the existing pattern used in #3423 was applied: ``` // TODO: [CmbServiceTests] Adapt to run with the service protected override bool UseCMBService() { return false; } ``` For classes that **do not** derive from `NetcodeIntegrationTest`, an added internal helper method was used: ``` [OneTimeSetUp] public void OneTimeSetup() { // TODO: [CmbServiceTests] if this test is deemed needed to test against the CMB server then update this test. NetcodeIntegrationTestHelpers.IgnoreIfServiceEnviromentVariableSet(); } ``` _(This PR includes some tests that includes one of the two script segments but was was reviewed and determined it didn't need to be run against a CMB server)_ ## Fixes Some issues were exposed in regards to the `NetworkClient` not being set to approved on all clients relative to each local `NetworkManager` instance when using the distributed authority network topology and connecting to a CMB server. ## Changelog NA - All internal testing and/or specific to testing. ## Testing and Documentation - Includes integration test adjustments. - Includes `NetcodeIntegrationTest` related adjustments. - Includes `NetcodeIntegrationTestHelpers` related adjustments. - No documentation changes or additions were necessary. ## Backport Does not require a backport since these changes are specific to updates in #3423 and over-all distributed authority integration testing.
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
3 participants
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.
MTT-11550
Changelog
Testing and Documentation
Backport
No backport applicable - DA only