open-project-1

open-project-1 copied to clipboard

Do you have a forum thread linked to this PR? https://forum.unity.com/threads/what-about-unit-testing.981069/ What will this PR bring to the project for everyone?

Setting up unit tests is a non trivial task within unity. Specifically creating a working Assembly Definition File requires an understanding of DLL references and increases project complexity.

The advantage once the tests are further fleshed out are two fold:

1. local automated of tests before the commit is pushed will allow developers to make changes without (as much) fear of breaking everything.
2. automated tests can be scripted to run on-commit via github actions - allowing direct and immediate feedback of a working build.

Why are these changes necessary?

Within the discussion on the thread the is some debate with respect to the value of testing and when to start. Based on my experience having tests in place from the early stages allows things to be developed in a testable way from the start. Adding testing later in the project lifecycle is MUCH more challenging.

How did you implement them?

In this instance I have created a proof of concept which will prove that tests can run within the context of the project.

I have stopped short of including a test of any of the monobehaviors because I'd like to get feedback on the decision to put the ASMDEF file in the root of the project versus having one for scripts and one for the input folder.

Additionally as a next-step goal I'd like to go through the effort of creating a MOCK and demonstrating a MOCK based test. On that front I'd also like to see some feedback on how the wider team would like to see the inclusion of NUGET packages handled. I know how I'd do it on my own, but there may be global preferences about that sort of thing.

I expect this PR will need more feedback - before being accepted - but it is a foundational piece to get the ball rolling.

All committers have signed the CLA.

Note the one thing that does concern me with this PR is found within the UnityOpenProject1.asmdef assembly definition file

Any libraries referenced in any scripts have to be included as references. So for example: using Cinemachine in one of the scripts, requires it be added to the ASMDEF. Without this definition file, it just seems to "work" based on unity-magic but with this in place every assembly has to be specifically referenced.

I feel like that does create a level of complexity that will be new to a lot of unity developers and possibly that perfect combination of unwelcome, annoying, frustrating and, stupid. I don't know if there is a solution to that outside of updated documentation.

Does anyone know of a way to auto-add referenced assemblies via ASMDEF?

AFAIK, there is no way to auto-reference every package with asmdef. The only way I see is not to use them, and just put all tests in Editor sub-folder.

Hmmm ... Interesting. Thanks for the insight. That'd be a pretty reasonable workaround, except the ASMDEF on the main project is required in order to be able to reference its code from within the context of the tests.

So for example, you can't actually create a "Protagonist" controller to run tests on it unless you can reference the assembly.

Update: I've put together a bit of reorganization.

• names for ASMDEF files are more fully qualified and specific for easier identification
• main SCRIPTS have been moved to their own ASMDEF rather than pointing all scripts to the root
• Auto Generated Input scripts have been placed in their own ASMDEF
• DLL packages and dependencies for using Moq within unit testing have been included and configured

Still outstanding:

• writing a test using Moq
• figuring out if testing the InputReader is even possible (unity forum thread: https://forum.unity.com/threads/question-about-testing-and-mocking-with-inputsystem.990848/)
• writing a README type file explaining the features of the unit testing setup for community education and understanding.

Ideally having a runnable unit test framework in place when things start coming in LIKE the finite state machine will make it easier to stay on top of testing and quality from the start of the project.

Per a forum comment, this PR adds 4 external dependencies each is listed here with a link to the licenses included

• https://www.nuget.org/packages/moq https://raw.githubusercontent.com/moq/moq4/master/License.txt

• https://www.nuget.org/packages/Castle.Core/ http://www.apache.org/licenses/LICENSE-2.0.html

• https://www.nuget.org/packages/System.Threading.Tasks.Extensions/ https://github.com/dotnet/corefx/blob/master/LICENSE.TXT (MIT license)

• https://www.nuget.org/packages/System.Runtime.CompilerServices.Unsafe/ https://licenses.nuget.org/MIT (MIT)

These license files have been extracted or downloaded as appropriate and included in the Assets/Plugins/UnitTesting.Mock folder

Just leaving some comments here instead of the forum post. Disclaimer: I have no clue about Unity but I'm somewhat good at unit/integration/functional testing and TDD/BDD/ATDD methodologies.

• Personally I'd suggest using Xunit, NUnit is a testing framework that has some vices which might encourage people to abuse them without knowing it (the Setup/Teardown pattern, for example, encourages contributors to fatten the Setup with initializations that are not necessary for other tests, Assert.Pass is an assert that tests nothing, instead it encourages programmers to add logic to tests, the Assert.Inconclusive method that marks a test as not passed but still passing it as okay, and the ThrowsNothing which tests no exception is thrown, which is already the default for every test line).
• Test names should be descriptive from the beginning, and I don't consider TestingTest as a descriptive name. I see that it's a sample test and that it just tests the framework (which I guess has already been tested by the NUnit folks) but people are bound to copy/paste from examples so you might end up with hundreds of "TestingTestXXXYYY" everywhere. Copying the name of the class into the test method is also a smell, you certainly don't want that as it will eventually turn into an enumeration of tests as TestingTest0, TestingTest1, etc.
• Personally (note that this is my personal way of writing names and I'm just offering it as an example) I use a class ending with Must (I've seen others use Should instead but it doesn't enforce the correct idea that tests must pass) along a descriptive method name that complements the class name. For example, I'd rename the first one to SimpleTestMust.PassImmediately, and would probably move the second test to its own class as you'd certainly not want people to mix different types of tests (unit tests must fly).
• Finally, if you're adding tests you should also modify the yaml file to run them automatically. These tests will never break though, which is why I suggest starting writing tests that actually test something instead of just placeholders.

Regards,

Hi , I have an example of testing input using the new input system. https://github.com/kgc00/BeatEmUpSandbox/blob/combat_v2/Assets/Scripts/TestSandbox/PlayMode/KeyLoggerTests.cs (more info here: https://docs.unity3d.com/Packages/[email protected]/manual/Testing.html)

Maybe I'll take a look tomorrow and see if there is a quick fix

Alternatively you could just not test the input, assume it works since it is unity core, and test the event systems / downstream code that uses it.

EDIT: Hi, i was able to get some Inputreader unit tests in. They're in this PR https://github.com/UnityTechnologies/open-project-1/pull/144 but feel free to use my approach here as well, Alex.