Skip to content

test: update rpc queue unit tests - #626

Merged
0xFA11 merged 19 commits into
developfrom
test/newrpcqueuetests
Mar 17, 2021
Merged

test: update rpc queue unit tests #626
0xFA11 merged 19 commits into
developfrom
test/newrpcqueuetests

Conversation

@NoelStephensUnity

Copy link
Copy Markdown
Member

This update to the RpcQueueTests provides the following functionality:
Improved names to better convey what aspect of the RpcQueue is being tested.
This includes a most likely undetected bug that would limit the maximum size that could be copied to 128kb within the NetworkWriter.ReadAndWrite method (primarily used by the RpcQueueHistoryFrame. NetworkWriter.ReadAndWrite now has no limits on the size.

Minor Adjustments to RpcQueueHistoryFrame:
Allowing the maximum Rpc Queue Message size to be adjustable. Currently it defaults to 1MB.

NetworkUpdateStagesComponent:
Used in conjunction with the RpcQueueTest to validate that Rpcs are being invoked at the proper NetworkUpdateStage

BufferDataValidationComponent:
Used in conjunction with the RpcQueueTest to validate from 1 byte to (n) MaximumBufferSize

The RpcQueue unit tests validate:
Maximum buffer size that can be sent from 1 byte up to 1MB (the default maximum RpcQueueHistoryFrame size)
That all RPCs invoke at the appropriate NetworkUpdateStage (Client and Server)
A lower level RpcQueueContainer test that validates RpcQueueFrameItems after they have been put into the queue

NetworkManagerHelper:
A nifty tool that now lives in the MLAPI.RuntimeTests namespace.
This makes writing MLAPI unit tests simpler while also making sure that if you fail it will clean up after itself.
Simplifies creating NetworkObjects and adding any unique testing component that are automatically cleaned at the end of the test(s). This includes a BuffersMatch helper method that will verify two buffers are the same!

This removes the NetworkManager.Singleton dependencies from within the RpcQueueContainer and RpcQueueProcessor classes.
This further strengthens the RPC Queue system as a whole, will allow for multiple instances if RpcQueueContainer, removes a limitation I set in NetworkWriter.ReadAndWrite,  and provides unit testing capabilities required for new RPC Queue unit Tests.
The new rpc queue tests without all of the extra files associated with them.
This is the merging artifact I had follow me through the branches.
This fix was inspired by Beck mentioning that the StartNetworkManager didn't have a comment that it could only start in Host mode.  I added some additional parameters to allow one to start the NetworkManager in Host, Server, or Client mode.
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs
Renaming m_CurrentNetworkManagerMode to s_CurrentNetworkManagerMode
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated

@becksebenius-unity becksebenius-unity left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/NetworkManagerHelper.cs
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/RpcQueueTests.cs

@0xFA11 0xFA11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe changes in RpcQueueHistoryFrame and the new NetworkManagerHelper should go under 2 separate, isolated, bite-sized PRs and then we should have this going into develop.

NoelStephensUnity and others added 5 commits March 16, 2021 17:31
Fixing camelCase.
Dropped a t in networkManagerComponent.  Added the t back.
A slicker version of my logging done in one pass (thanks to Fatih for the suggestion)!
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/RpcQueueTests.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/RpcQueueTests.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Tests/Runtime/RpcQueueTests.cs Outdated

@0xFA11 0xFA11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 (with some minor edit suggestions)

@0xFA11
0xFA11 merged commit 8a5ab8b into develop Mar 17, 2021
@0xFA11
0xFA11 deleted the test/newrpcqueuetests branch March 17, 2021 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants