Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: base changes to PR-1114 #1165

Merged
merged 14 commits into from Sep 14, 2021
Merged

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Sep 10, 2021

The base requirements to build and run multiprocess tests with a minor adjustment to the KillProcesses method.
This PR disables the Multiprocess tests by default, but they can be enabled while continuing development by including the following command line argument:
-bypassIgnoreUTR

The base requirements to build and run multiprocess stuff.
removing LF
Adding back the multiprocess ignore capabilities with the ability to easily override this ignore for continued development work.
including the IgnorMultiprocessTests test check for the NetworkVariablePerformanceTests
fixing white space formatting issue due to indentation.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review September 13, 2021 20:22
@zain-mecklai
Copy link
Contributor

These changes look fine to me. I have to admit I don't understand the implications of everything going on here so I can't provide a more detailed review.

Copy link
Contributor

@zain-mecklai zain-mecklai left a comment

Choose a reason for hiding this comment

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

I tested them out and they appear to have solved all the problems.

Comment on lines 813 to +815
}


#if UNITY_EDITOR || DEVELOPMENT_BUILD
#if UNITY_INCLUDE_TESTS
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not sure how this macro works and I wouldn't advise to change it if not necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is defined in the com.unity.netcode.runtimetests.asmdef file:
"defineConstraints": [
"UNITY_INCLUDE_TESTS"
],
According to @JesseOlmer it is the preferred define to use for this particular scenario.

@NoelStephensUnity NoelStephensUnity merged commit 0941fd7 into develop Sep 14, 2021
@NoelStephensUnity NoelStephensUnity deleted the test/multiprocess-fixes branch September 14, 2021 22:17
SamuelBellomo added a commit that referenced this pull request Sep 16, 2021
…nsform

* develop:
  feat: NetworkBehaviour.IsSpawned  (#1190)
  feat: added tip to the network manager inspector that directs to install tools (MTT-1211) (#1182)
  refactor!: remove network dictionary & set, use native container in List, add tests (#1149)
  fix: Fixed remote disconnects not properly cleaning up (#1184)
  test: base changes to PR-1114 (#1165)
  test: verify do not destroy networkobjects on networkmanager shutdown (#1183)
  chore: removal of EnableNetworkVariable in NetworkConfig. It's always True now (#1179)
  fix: Fix DontDestroyWithOwner not returning ownership (#1181)
  test: Giving Android some more room as the connection tests are timing sensitive (#1178)
  fix: unitytransport connectionmode buttons (#1176)
  test: added min frames to multi-instance helper (#1170)
  chore: Add mobile tests to nightly trigger (#1161)
  feat: snapshot spawn pre-requisite (#1166)
  feat: Unity Transport + Relay (#887)
  feat: client scene synchronization mode (#1171)

# Conflicts:
#	testproject/Assets/Scenes/SampleScene.unity
SamuelBellomo added a commit that referenced this pull request Sep 16, 2021
…am/feature/client-network-transform

* sam/feature/interpolation-for-network-transform: (22 commits)
  fixing line issue
  more formatting
  fixing formatting issue
  removing not submitted LiteNetLib from ZooSam
  feat: Fast buffer reader and fast buffer writer (#1082)
  restricting public api
  bumping exec order
  feat: NetworkBehaviour.IsSpawned  (#1190)
  feat: added tip to the network manager inspector that directs to install tools (MTT-1211) (#1182)
  refactor!: remove network dictionary & set, use native container in List, add tests (#1149)
  fix: Fixed remote disconnects not properly cleaning up (#1184)
  test: base changes to PR-1114 (#1165)
  test: verify do not destroy networkobjects on networkmanager shutdown (#1183)
  chore: removal of EnableNetworkVariable in NetworkConfig. It's always True now (#1179)
  fix: Fix DontDestroyWithOwner not returning ownership (#1181)
  test: Giving Android some more room as the connection tests are timing sensitive (#1178)
  fix: unitytransport connectionmode buttons (#1176)
  test: added min frames to multi-instance helper (#1170)
  chore: Add mobile tests to nightly trigger (#1161)
  feat: snapshot spawn pre-requisite (#1166)
  ...

# Conflicts:
#	com.unity.netcode.gameobjects/Components/NetworkTransform.cs
#	testproject/Assets/Scenes/SampleScene.unity
mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
* Refactor

The base requirements to build and run multiprocess stuff.

* style

removing LF

* fix

Adding back the multiprocess ignore capabilities with the ability to easily override this ignore for continued development work.

* fix

including the IgnorMultiprocessTests test check for the NetworkVariablePerformanceTests

* style

fixing white space formatting issue due to indentation.

* Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/BaseMultiprocessTests.cs

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>

* fix

spelling of IgnoreMultiprocessTests

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
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.

None yet

3 participants