in0finite / SanAndreasUnity

Open source reimplementation of GTA San Andreas game engine in Unity
https://discord.gg/p6jjud5
MIT License
2.15k stars 353 forks source link

Unit testing and static object issues #35

Closed a-ivanov closed 4 years ago

a-ivanov commented 4 years ago

Project doesn't have any unit tests. There are numerous well-known benefits with them. Unity has NUnit integration with specific Play mode.

When I tried to implement a test I found it difficult due to global static variables and singletons couplings.

Example: test Ped.SpawnPed(int, Transform). It calls Item.GetDefinition(int) which searches a static Dictionary for preloaded PedestrianDef.

Such things prevent mocking (e.g. with Moq).

There are alternatives to singletons like Service Locator pattern (a simple Dependency Injection) which is employed in other projects. It is not perfect but in contrast to singletons it allows us to write quality tests with mocks and decouples concrete service implementations.

What do you think of gradual migration to such components structure?

in0finite commented 4 years ago

I am aware of benefits of unit testing. But there are many singletons in the project, and migrating to other solutions would require a lot of work. So, my opinion is that one should focus on implementing features/fixing bugs, rather than on making tests.

slapin commented 4 years ago

Usually implementing tests together with features have long term benefits. Just saying. In general nobody likes doing tests until last moment, so this have to be experienced to understand. Usually implementing tests post-factum is very hard task to do though; planning upfront makes test writing semi-automatic with proper architecture. So if you don't do it from beginning it will be no fun to start now.

On Tue, Sep 24, 2019 at 3:07 AM in0finite notifications@github.com wrote:

I am aware of benefits of unit testing. But there are many singletons in the project, and migrating to other solutions would require a lot of work. So, my opinion is that one should focus on implementing features/fixing bugs, rather than on making tests.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GTA-ASM/SanAndreasUnity/issues/35?email_source=notifications&email_token=AAABPU65IOKROJENAWYUVKDQLFK3BA5CNFSM4IZUAEP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7MUGXY#issuecomment-534332255, or mute the thread https://github.com/notifications/unsubscribe-auth/AAABPUYOQ22FQVRZC2S7LMLQLFK3BANCNFSM4IZUAEPQ .

a-ivanov commented 4 years ago

To tell you the truth, I have started working on this. The project is a promising one and fun, but also a big one. The code quality is gonna degrade without testing.

It also reveals code smells which translate to bugs and a lack of inner mechanics understanding by others.

a-ivanov commented 4 years ago

The main problem with testing now is that all game objects and usual C# classes are interconnected via static singleton / functions. It is not practical (and maybe impossible sometimes) to test such code.

The first thing to do is to find a way to decouple dependencies in order to substitute them in tests. As long as Unity creates game objects this is a tricky business but there are ways I'm trying to do / researching now.

in0finite commented 4 years ago

I wouldn't like to dissapoint you. But my opinion is that tests are not neccesary at this moment. We should focus on high priority stuff. Doing what you want requires changing core parts of the game, which can cause a lot of bugs.