Improvement on ProtocolGame testability and unit test coverage

[danilopucci]

Explanation of what you want to do that is currently impossible

I was trying to make an optimization on ProtocolGame and before do it I was trying to add a unit test to this class. First to increase its coverage and guarantee that my optimization will not break anything and second to have a easy way to benchmark it.
The problem is that this ProtocolGame class is coupled to some other singleton objects, which makes it kind of complex to test this class.
I think this is a very important class to add on unit tests just beucase it may be the ones that tends to change over the time, since the protocol is always evolving and changing.

Desired functionality

The desired functionality would be to do not depends directly on this singletons objects, this way we can inject its dependecy on the code and on unit test code we can mock this objects.

Available workarounds

The desired functionality would need a decoupling on the singletons that this class has: g_dispatcher, g_game, g_chat, g_creatureEvents etc.
A workaround or maybe the way to go to achieve this is to do a dependency injection of the mentioned singletons.
Also, would be necessary to implement interfaces to this mentioned class objects, so it would be possible to implement mocked objects to fit the test cases.

Before work on this, I have to ask the question:

To improve testability and unit testing is something that is desired on this project?
To add a dependency injection here and in other classes, is something that is acceptable on the project architecture?
Is there any approach that is followed?

[Zbizu]

To improve testability and unit testing is something that is desired on this project?

It's something everyone wants, but no one really works on

To add a dependency injection here and in other classes, is something that is acceptable on the project architecture?

If you provide a small sample someone should reply with a review

Is there any approach that is followed?

Check src/tests folder

[lgrossi]

A workaround or maybe the way to go to achieve this is to do a dependency injection of the mentioned singletons. Also, would be necessary to implement interfaces to this mentioned class objects, so it would be possible to implement mocked objects to fit the test cases.

I wouldn't necessarily call it a workaround. I consider it a valid solution/approach, but it won't solve all the problems we face to run tests in the current state of the code base. In canary we did implemented a DI and some stub mechanism to remove the singletons dependencies on tests.

That does improve a bit the tests, however you would still have a bunch of annoyances from the code design (very coupled flows, god-like objects that are shared all over the place and are instantiated with a bunch of coupled behaviours, including with data sources and such, etc, etc).

I guess the protocol files suffers a little bit less from that, because if you can isolate it from the singletons and abstract away the tcp connection, you can pretty much test the output functions of the protocol and see if they build the expected/proper message. The input is a bit more annoying because you would need to test if a given input message builds the proper objects and call the proper flows. While the later is relatively easy with DI, the more complex objects are quite challenging to instantiate standalone because of tons of coupling.

In the end of the day, to have a true functional test environment in the code bases one would need to do a considerable refactor of the main flows, specially the badly designed inheritances, the coupling between representations and data layers and lack of defined responsibilities/boundaries between different systems and entities of the game state.

note: it's worth mentioning that the DI libraries in CPP are also not that great, they are far behind some more elaborated DIs like symfony-containers in PHP and the spring beans in Java.

Reference (they are mostly old but gives the general idea):
first trial with DI
some tests leveraging runtime injection override (valid to mention that we use a different UT framework, a small one not standard, but that I personally find quite easy to use).

[danilopucci]

Cool, thanks for your suggestions!

I think your/canary solution is great if I was aiming for a full functional tests, but as (at least for now) I want just some unit tests on ProtocolGame will try to keep it simpler.

I am saying this only with some draft (but working) code, maybe I regret 🤣