With everything going on I thought it might be nice to revisit the fast paced world of continuous integration, static analysis and architectural ratcheting. I’m thinking back to those halcyon days of architecture review meetings where adding a single new method could justify a full day of deliberation.
Today we have so many new fellow developers who are all so very busy and so fantastically productive. I sadly don’t think our esteemed new colleagues will be attending our architecture reviews so in an attempt to modernise our approach we should bring the architecture reviews to them.
Instead of stacking endless guidance and warnings we can be more reactive and targeted in our feedback using just a few simple CI patterns. The goal is to no longer allow the functional outcomes to be achieved at any cost but rather to force their commits to sit within the project’s established architecture constraints and NFRs.
Prior art:
Pattern One - Targeted Feedback via CI
When was the last time you had a look at the breadth of things you can define in a test framework? Building Evolutionary Architectures calls these fitness functions: tests that check the shape of the code rather than just what it does.
Just as a small sample of delights:
Wall off dependencies
.ShouldNot().HaveDependencyOnAny(new[] { "UnityEngine", "UnityEditor" })
Push back on lazy naming
.ShouldNot().HaveNameEndingWith("Manager")
Stop cookie cutter solutions from spreading
.Should().MeetCustomRule(new NoSelfTypedStaticField())
Put a limit on comments
DocCommentBudget.Offenders(DocCommentBudget.SourceRoot())
These rules wouldn’t all need to be known and understood by our verbose friends. They would surface in response to whatever task was being attempted.
Pattern Two - Test Repository Segregation and Ratcheting
If one was so inclined they could segregate their tests into a separate repository with its own access controls. Doing this would of course create a second class of programmer who is bound by tests they’re unable to edit. This doesn’t have to be a dead end without recourse, each test can point to its own git issue where appeals can be made (and aggregated).
Surfacing the tests for them is the easy part, how you ensure they are pulled from the test repo will vary by the specifics of your codebase (e.g. packages for Unity or a git submodule).
A common problem with fitness function test suites is that they’re impossible to expand without first fixing every existing violation. You’re left trapped by past sins. This is where ratcheting comes in: it enforces the rules only for new code and grandfathers in the existing codebase.
Again the details will vary, NetArchTest has no built-in freeze but ArchUnitNET does via FreezingArchRule if you’d rather not roll your own. Tools like reviewdog will filter linter and test results to only the lines in the diff. Those with access to the test repo will be free to add new exceptions and rules as needed.
Pattern Three - Build Server Enforcement
All of this so far is still a serving suggestion. We can communicate the rules and state that passing the test is mandatory but a determined programmer will eventually find a creative justification for why this time it’s different. Unburden them from that temptation, we can’t enforce the tests client side but we can do so trivially on a build server.
Make sure our determined programmer is fully aware of the build server but protect the exact details of its configuration and how it double-checks the tests are running properly. At a minimum this means the build server should fetch its own copy of the test suite, be able to block merges that fail and have some kind of baseline for how many tests it should contain and take to run.
Lastly, it should be made clear that a clean bill of health from the build server is always part of the scope of work. Knowing a task isn’t finished until the build server says so makes sure they can’t claim a false win and exit early.
Rules for thee, not for me
Apply all three and you have a suite of tests that holds those who don’t have access to the test repo to a different set of standards than those who do. The fact these tests are mandatory is clearly communicated and so is their unavoidability. No creative workaround or even simple --no-verify shortcut gets past the build server.
Our esteemed colleagues lose nothing they will miss and they have all the time in the world to figure out how to comply. Each bit of feedback arrives only when needed and nudges them iteratively back into compliance. If the walls hold then there will be no more sweeping refactors pitched as novel features and waved through on a quick ‘looks good to me’.
Header image: detail of Siege of a city, a painted miniature from a copy of Ogier le Danois (Antoine Vérard, Paris, 1496–1499), Biblioteca Nazionale, Turin. Unknown artist. Public domain, via Wikimedia Commons.