test: manual rpc tests to automated fixed revision #841
Conversation
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
This is the first pass version with a duplicated MultiInstanceHelper. This includes SIPTransport updates that prevent issues with multiple SIPTransport unit tests as the S_Server was never getting reset. So, upon the host-server disconnecting it resets the value back to null so the next unit test can initialize the SIPTransport again. This includes a fix for unit tests that may result in the SnapshotSystem attempting to send before m_NetworkManager is set.
…/Unity-Technologies/com.unity.multiplayer.mlapi into test/manualRpcTests-to-Automated
…rt.cs Co-authored-by: Albin Corén <[email protected]>
…rt.cs Co-authored-by: Albin Corén <[email protected]>
Co-authored-by: M. Fatih MAR <[email protected]>
…rt.cs Co-authored-by: Albin Corén <[email protected]>
Co-authored-by: M. Fatih MAR <[email protected]>
Now uses frame count to detect a timeout as opposed to using time in the event a virtual machine, during Yamato testing, is under a heavy load and frame per second are much lower than expected. This also includes changes in the RpcQueueManualTests to help reduce the time a full test will take.
Did some final clean up on the code to prepare for review. Cleaned up/added comments. Added an Application.targetFrameRate check that will assure this test, for all platforms, runs at the same frame update rate.
| /// <param name="server">The server</param> | ||
| /// <param name="result">The result. If null, it will automatically assert</param> | ||
| /// <param name="maxFrames">The max frames to wait for</param> | ||
| public static IEnumerator WaitForClientsConnectedToServer(NetworkManager server, int clientCount, CoroutineResultWrapper<bool> result = null, int maxFrames = 64) |
There was a problem hiding this comment.
Why do we have a duplicate method for this? Why not just have a default clientCount=1?
There was a problem hiding this comment.
Good point, I just made some adjustments which will remove the duplicate code while still keeping the methods in-tact.
( Albin was the original author so I will leave it up to him to determine if we should completely remove the "wrapper methods")
| /// <param name="result">The result. If null, it will automatically assert<</param> | ||
| /// <param name="maxFrames">The max frames to wait for</param> | ||
| /// <returns></returns> | ||
| public static IEnumerator WaitForClientsConnected(NetworkManager[] clients, CoroutineResultWrapper<bool> result = null, int maxFrames = 64) |
There was a problem hiding this comment.
This seems like a lot of duplicated logic, isn't there a way to reuse the other WaitForClientsConnected methods?
There was a problem hiding this comment.
Good point, I just made some adjustments which will remove the duplicate code while still keeping the methods in-tact.
( Albin was the original author so I will leave it up to him to determine if we should completely remove the "wrapper methods")
0xFA11
reviewed
May 20, 2021
| } | ||
|
|
||
| /// <summary> | ||
| /// ***Should always be invoked when finished with a single unit test*** |
There was a problem hiding this comment.
Suggested change
| /// ***Should always be invoked when finished with a single unit test*** | |
| /// Should always be invoked when finished with a single unit test |
0xFA11
reviewed
May 20, 2021
| int startFrame = Time.frameCount; | ||
|
|
||
| while (Time.frameCount - startFrame <= maxFrames && !client.IsConnectedClient) | ||
| var startFrame = Time.frameCount; |
There was a problem hiding this comment.
Suggested change
| var startFrame = Time.frameCount; | |
| var startFrameNumber = Time.frameCount; |
0xFA11
reviewed
May 20, 2021
| } | ||
|
|
||
| int startFrame = Time.frameCount; | ||
| var startFrame = Time.frameCount; |
There was a problem hiding this comment.
Suggested change
| var startFrame = Time.frameCount; | |
| var startFrameNumber = Time.frameCount; |
0xFA11
reviewed
May 20, 2021
0xFA11
approved these changes
May 20, 2021
…om/Unity-Technologies/com.unity.multiplayer.mlapi into test/manualRpcTestsToAutomated-Fix
SamuelBellomo
added a commit
that referenced
this pull request
May 21, 2021
…player-object-get * develop: test: manual connection approval to fully automated connection approval testing (#839) test: manual rpc tests to automated fixed revision (#841) fix: CleanDiffedSceneObjects was not clearing PendingSoftSyncObjects (#834) feat: NetworkTransform now uses NetworkVariables instead of RPCs (#826) fix: gracefully handle exceptions in an RPC invoke (#846) fix: Fixing utp license file for legal (#843) ci: enable standards check on UTP too (#837) refactor: NetworkBehaviour.NetworkObject no longer throws an exception (#838) fix: revert #830 (#840) test: converting the manual rpc tests over to an automated unit test (#830) # Conflicts: # com.unity.multiplayer.mlapi/Tests/Runtime/MultiInstance/MultiInstanceHelpers.cs
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.
This work is based off of MTT-764.
It includes the conversion of the manual RpcTests over to an automated unit test with 1 Host and 7 Clients (8 clients total).
Summary
This task is to convert the manual RpcTesting tests over to an automated unit test leveraging from the new MultiInstanceHelpers class. This includes the refactoring of the RpcQueueManualTests component to be able to handle both Manual and Automated testing.
Acceptance Criteria
The unit test should perform at least one full cycle of the various RPC tests performed by the manual test using 1 Host and 7 clients for a total of 1 Server and 8 clients. The RPC tests performed can be verified by the final output of the unit test to assure all runtime RPC related counters reflect that each type of test has been counted for each client and server multiple times (it varies depending upon the test type). The only value that will not be conveyed is the progress for each multi-client striped test. That is verified by the fact that the test completed successfully.