1

Let's say in an effort to improve quality, your team agrees on the following policy:

  • all commits to the master branch (direct or via pull request) have to be categorized as either bugfixes or feature changes; never both at the same time.

  • to easily distinguish the two, commit messages have to start with e.g. [bug 123] or [feature 456], respectively

  • in feature commits, all code changes have to be feature-flagged (using some sort of application settings tool such as NFig). The idea is that while the feature flag is turned off, the commit is not supposed to change app behavior in any way.

EDIT: feature-flagged are supposed to be short-lived and only for release purposes. Martin Fowler calls those release toggles.

So for example, a feature commit like this...

function existingCode() {
   // existing stuff

+   doSomethingNew();  // <--- NEW 

   // more exiting stuffstuff
}

... would be in violation of the policy because doSomethingNew() introduces an immediate change in behavior. Feature-flagging the change like this...

function existingCode() {
   // existing stuff

+  if (Config.FeatureFlags.NewFeature456)
+  {  
+    doSomethingNew();  // <--- NEW 
+  }    
   // more exiting stuffstuff
}

... would be compliant with the policy: there is no change in behavior until the NewFeature456 feature flag gets enabled (and therefore no risk of accidental breakages, feature leaks etc).

My question is: would it be conceivable to enforce the feature flag policy with an automated test?

The way I would imagine a test like this to work is, roughly:

  • inputs: git repo, commit hash, name of feature flag
  • build repo with and without commit (feature flag disabled in both)
  • check if both builds behave the same

That last "behaving the same" part is obviously the tricky part. A number of things come to mind here, from running a number of other automated tests to code analysis - something like comparing the ASTs of both builds (that might get difficult).

Could that possibly work? Has it been done before?

  • 1
    seems like it would be fairly easy, just mark up your tests against a feature flag and run the enabled ones. The trouble with feature flags is that you soon have infinite combinations of tests to run – Ewan Jul 29 '18 at 6:33
  • all commits to the master branch there is your problem. NEVER commit to the master branch. ONLY EVER merge to master from a branch that, prior to merging, has undergone a full test-cycle. – marstato Jul 29 '18 at 8:36
  • 1
    If you have this policy, how can you ever remove a feature flag for a feature that has been successfully tested and you have decided to keep permanently? Presumably you don't want every feature in the application to be permanently configurable. – bdsl Jul 29 '18 at 10:49
  • Feature flag is a good tool. However, like other good things, overusing them would be harmful. – ivenxu Jul 30 '18 at 1:55
  • Edited for clarity: feature-flags are supposed to be short-lived, not permanent. – Max Jul 30 '18 at 14:24
1

I think you totally miss the root cause of your problem. I guess your real porblem is this:

want features to only become active when someone who is not a developer (management?) decides that now is the right time.

The solution, IMO, is NOT to clutter your code with distracting and leaky/incorrect if (featureIsEnabled) checks and weld some wanky static code analysis on top.

Your solution is to use your VCS correctly. You say master branch so i assume you are using Git. When using git, your best option is to use one of the standard branching models (e.g. GitFlow). What all proven and widely-used git branching models have in common is this:

all commits to the master branch there is your problem. NEVER commit to the master branch. ONLY EVER merge to master from a branch that, prior to merging, has undergone a full test-cycle.

If you use e.g. GitFlow you can have your feature branches ready. You can test them to your hearts content and make sure they stay integrated with the main branch as that moves on. Then, when someone decides the feature should become active, you do another integrate&test cycle on the feature branch, then merge it.
When a particular feature needs a feature flag (e.g. to only activate for certain customers / on certain system settings), that should be part of the feature. Test that flag as if it was part of the feature (just like when i click that button, the foo is frobnicated).


If, however, your requirement is that all features should be possible to enable or disable at will, you're in BIG trouble. That is a problem your entire organization must be ready for: with every new feature the possibility of system states is multiplied by (at least) 2. Do you have test cases for all possible combinations of enabled features? Are you prepared for the massive overhead this encurs on development and release? Do features depend on each other and if so, do you have mechanisms in place to prevent a feature from being enabled while its dependencies are not?

Also, if you really want to go this route, you should consider a module system rather than putting all of the code into one thing. Take a look at how e.g. modern CMSs handle modules/plugins.

| improve this answer | |
  • 1
    good points re plugin systems and that feature toggles can get excessive. But your recommendation to delay feature branch integration is a total git antipattern – to which feature toggles are a potential solution. The problem is that as the branches diverge over time, the effort to integrate them and resolve merge conflicts increases. Keeping features small and integrating early is the best known way to avoid such problems. – amon Jul 29 '18 at 9:57
  • 2
    @amon I'd like to challenge your statement. Unless you have automated test suites for all possible feature flags (major effort, 2 to the n where n is the number of flags), you have the same problem: feature gets merged with toggles. Two months later someone decides to actually toggle the feature on. The feature is buggy. Why? Other things changed and noone re-tested. – marstato Jul 29 '18 at 14:49
1

I think the only way to do this is to have extensive testing for all the existing features, including testing things like the content of menus to make sure no new unexpected items show up.

If the application has a GUI, then some integration tests that make screenshots of the GUI and compare them to reference screenshots in the repo might make this a bit easier. There are tools that will check for similarity if 100% binary identical images are not feasible. More generally you can compare any output of the application to a reference example.

You might then be able to enforce a rule that a new feature commit doesn't change any of the existing integration test files, but to be honest I think you could probably do this as well by code review as by an automated system.

I don't think you'd want to use code analysis of the production code as a check for this. If you did that, you could ensure that other than the feature-switched-on branches of the code everything is the same as before, but then you'd end having to duplicate any code which would need refactoring or extending to support the new feature.

You'd delay any bugs caused by mistakes until the time the feature is switched on. Delaying bugs is a very bad thing - if bugs are going to happen the sooner they do the better since they will be much easier to fix. In fact if you're using if statements to separate out the new code from the old code you're not really integrating, and you might as well have long-lived feature branches in the VCS. It's much better to let the new code run straight away, and just hide the result of it at the UI layer.

As I suggested in my comment, I think you also need a third type of commit. Feature flags are tech debt if you have too many in an application at once. So you need to make commits that remove feature flags, by going through the codebase and replacing a value like Config.FeatureFlags.NewFeature456 with either true or false, and then refactoring.

| improve this answer | |
-1

If your language supports parsing to AST, it sounds like relatively simple code analysis problem. For each changed file, parse it to AST before and after change, then descent it concurrently, until you hit the difference, skipping over "if(feature) { }" statements.

It may have some nuances, for example you probably want to allow declaring variables with default values so that they could span over several "if (feature) {}" statements, but overall seems doable

| improve this answer | |
  • You'd probably want to allow declaring new symbols (variables, constants, classes, functions, methods etc) in general, not just variables. But even so, if you got such an extent of segregating new from old code via if statements I think you'd miss out on most of the benefits of integrating it in the first place and you might as well have left it on a feature branch. – bdsl Jul 29 '18 at 11:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.