test: converting the manual rpc tests over to an automated unit test - #830
Conversation
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.
| } | ||
|
|
||
|
|
||
| public static void ShutdownAndClean() |
There was a problem hiding this comment.
minor: I'd name this as Destroy to keep it similar to Create/Start formats.
| public static void ShutdownAndClean() | |
| public static void Destroy() |
There was a problem hiding this comment.
I really don't have a preference to what we name this...
@TwoTenPvP What do you want this to be named?
| "Unity.Multiplayer.MLAPI.Runtime", | ||
| "UnityEngine.TestRunner", | ||
| "UnityEditor.TestRunner", | ||
| "Unity.Multiplayer.MLAPI.RuntimeTests", | ||
| "TestProject.ManualTests" |
There was a problem hiding this comment.
changes could be limited to only 2 added lines and the rest of the diff should not be here (might require manual text edit to prevent Unity's auto gen):
| "Unity.Multiplayer.MLAPI.Runtime", | |
| "UnityEngine.TestRunner", | |
| "UnityEditor.TestRunner", | |
| "Unity.Multiplayer.MLAPI.RuntimeTests", | |
| "TestProject.ManualTests" | |
| "Unity.Multiplayer.MLAPI.Runtime", | |
| "Unity.Multiplayer.MLAPI.RuntimeTests", | |
| "TestProject.ManualTests" |
also, Testproject.RuntimeTests depending on TestProject.ManualTest + MLAPI.RuntimeTests doesn't sound right to me.
we started to go a little bit crazy in dependencies I think :D
is this because we're not organizing code better or are these dependencies inevitable?
There was a problem hiding this comment.
Did you test this change before you suggested it?
There was a problem hiding this comment.
here you go:
{
"name": "TestProject.RuntimeTests",
"references": [
"Unity.Multiplayer.MLAPI.Runtime",
"Unity.Multiplayer.MLAPI.RuntimeTests",
"TestProject.ManualTests"
],
"optionalUnityReferences": [
"TestAssemblies"
]
}There was a problem hiding this comment.
Nice suggestion...
"optionalUnityReferences": [ "TestAssemblies" ]
Assures the test assemblies will be loaded:
"UnityEngine.TestRunner",
"UnityEditor.TestRunner",
| private void Update() | ||
| { | ||
| if (NetworkManager.Singleton && NetworkManager.Singleton.IsListening) | ||
| if (NetworkManager != null && NetworkManager.IsListening && ((IsServer && NetworkManager.ConnectedClientsList.Count > 1) || IsClient) && m_ContinueToRun && m_BeginTest) |
| m_ClientProgressBar.fillAmount = Mathf.Clamp((2.0f * (float)m_GlobalDirectCounter) * k_ProgressBarDivisor, 0.01f, 1.0f); | ||
| if (m_ClientProgressBar && m_ClientIndices.Contains(NetworkManager.LocalClientId)) | ||
| { | ||
| m_ClientProgressBar.fillAmount = Mathf.Clamp((2.0f * (float)m_GlobalDirectCounter) * k_ProgressBarDivisor, 0.01f, 1.0f); |
There was a problem hiding this comment.
an innocent operation like this takes a long way in branches to be executed :)
I feel like there could be a logic compression somehow but maybe all those checks are really necessary.
There was a problem hiding this comment.
Please take note of the !UnitTesting on line 778
There was a problem hiding this comment.
This was a manual test that is now a hybrid:
- It can operate in manual mode
- It can operate in unit test mode
It will be fine. :)
There was a problem hiding this comment.
It saves fuel of course!
There was a problem hiding this comment.
in the scope of this PR, I'd appreciate that but this reminds me that we need to have a "dependencies and isolation" conversation soon, internally.
There was a problem hiding this comment.
In all reality, it was made hybrid so we can perform a visual acceptance test on the acceptance tests. This is why the end of the unit test it logs the count totals for everything, so one can verify that indeed the counters are all being incremented in the same fashion running as a unit test as it does when ran "manually".
There was a problem hiding this comment.
if the test was to be fully-automated, it wouldn't need manual testing or visual testing in the first place :)
There was a problem hiding this comment.
Well perhaps the task was not defined properly... it was to convert the manual tests over to unit tests.
(that leaves a lot of room for interpretation)
…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]>
|
apparently, this PR breaks Ubuntu tests on |
* develop: 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)
…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
3 participants
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.