I’m pretty sure BMO was designed to make module development easier for third party developers first and unit tests seem to be an after thought. But, this is okay, BMO allows FreePBX to be modular and enables allows it to be extended really easy. I personally believe that BMO was one of the best decisions the FreePBX project made. I’m not sure how I would have done thing differently. That being said, there are things I personally believe shouldn’t have been made into BMO modules and should just be part of framework, such as config and database. This may have changed a few things, like… having a framework depedency and a module registry dependency instead of having the single FreePBX dependency that we have today.
My biggest gripe is that the registry pattern we used makes code harder to test, since all tests need to interact with the same class to set the fake dependencies of a class under test. And this is the problem.
To resolve this, we will probably need to create a ton of classes that simulate the FreePBX application, just so we can test it easy. This will be a TON of work.
As for the old code being deprecated, I totally agree, we shouldn’t be adding or modifying code that can’t be tested with unit tests. But, our current situation is that we’re in a place where it’s too hard to write unit tests to begin with and until we resolve that problem, things will just be as they are.
Respectfully Matt. @xrobau is the one who wrote and designed BMO. He’s the one who convinced the team and team lead (@plindheimer) at that time to move forward with it and let him write it. So he’d be the ultimate authority on what it was designed to do. (No hard feelings or I’ll will here I just think credit should be due to the person who created it and helped maintain it until his departure in late 2018)
Since I worked on it with Rob (and additionally so did Bryan Walters, Jason Parker and James Finstrom) I can say it had nothing to do with third party developers and everything to do with moving from procedural functions to object orientated functions which actually reduces memory consumption due to the fact that modules can be loaded “on-demand” instead of losing all procedural functions at run time (loading all functions.inc.php). Procedural functions are almost untestable. Object based functionally is way more easily tested. My 2 cents, implementing BMO was the starting path for the ability to actually do unit tests.
BMO in essence is a dependency injection container. It’s the same theory of thought that many large php projects use like Laravel and Symfony.
It might look like BMO is there to help third party developers but I would say that’s thanks to @jfinstrom and his work on developer starting points which utilize a lot of BMO
I’m not saying that @xrobau and the former developers don’t deserve a lot of credit for what was done in the past, they do.
Thanks for clearing up the design decisions for WHY BMO was created. Maybe the intent was for better unit testing, and maybe we got there some, but we’re definitely not where we need to be and there are still roadblocks to having really good unit testing support in the project.
What I can tell you is that both procedural functions and object oriented code CAN be easy to test, but the code has to be written with unit tests in mind. Just because something is written with Object based functionality doesn’t make it any easier to test. The FreePBX BMO code was NOT written with unit tests in mind, if it was, then all of the work would have great unit test support, but it does not. I’m simply stating where we are as a project.
Here are some of the questions I think about a lot and will ask you all in regards to how we could improve unit testing in BMO:
Is there something that can be done so that we don’t have to wrap the phpunit binary so that it loads the entire FreePBX to run tests? I’m sure the phpunit team knows more about unit testing than all of us, so we should probably strive to fit into their framework rather than work around it. Example: A solution could be splitting the dependency management into a thin module available in composer.
Can we make it so module unit tests do not require the FreePBX app to be loaded to test? Basically, can we simply run ‘phpunit’ from the module’s home directory and just have ‘it work’?
What can we do better to ensure that tests artifacts created in the test environment don’t leak over into the dev environment like it does today?
I belatedly realised that this question was probably addressed to me!
What needs to be done is that the code that I originally wrote, (8 years ago!!) needs to be updated to use Traits, which will then remove a lot of the ancient code that I had to hack in there to support the legacy codebase.
Then you just need to tell bootstrap to not load anything, and you’ll have a pristine FreePBX::create() that you can then inject mock modules into, including replacing the Database object if you want.
I’m pretty sure a chunk of this is in the unit tests in framework already. There’s no rocket science here, but you’ll get some insights by looking at the history of BMO (I like to turn ‘blame’ on, like in the first link, and then read the individual commits) as to why things have been done in that specific way - I did try hard to put the reasons for design decisions into the commits.