5 ms·

I can perhaps understand where this one is coming from: I refactored the code from untested and untestable, to testable with 40% test coverage. The senior arch

I can perhaps understand where this one is coming from: I refactored the code from untested and untestable, to testable with 40% test coverage. The senior architect is refusing to merge because the test coverage is to low. You have a piece of software running in production; even if it's poorly written and bereft of tests, your customers are satisfied. Customers (unwittingly) have already vetted this version! A rewrite, even with brilliant design and fantastic test coverage, is still more of a risk than fixing what isn't broken. This isn't to say that you shouldn't rewrite code, especially if it makes other improvements possible. But don't discount how high a bar new code has to reach to measure up with "it already works". Maybe 40% is still too risky.

AstralStorm 7y ago
It's always a lie, about running in production. I've personally seen untested code allegedly running but it isn't actually working. Defect numbers through the roof and test harness literally not being merged exactly because of this issue. You will get visibility, either by customer complaints, slowing down development speed and ultimately projects being shut down. It's not sabotage and it's not exactly drowning. It's willful attempt to ignore reality and cheat on metrics, prolonging the inevitable and defrauding stakeholders.
- BigJono 7y ago
  It's always a lie? You're basically arguing that code with bad test coverage never works. Production code is copping constant manual end-to-end tests. Those tests are more valuable than most unit testing, and some automated black box testing. I'll take stable production system over the unit tested new code any day of the week.
  - peterkelly 7y ago
    That's the kind of thinking that leads to situations like this: https://www.digitaltrends.com/mobile/samsung-galaxy-s10-ultrasonic-fingerprint-scanner-fooled-by-screen-protector/ https://www.digitaltrends.com/mobile/samsung-galaxy-s10-ultr... A lot of the time it's just as important to make sure certain things can't work as making sure that certain other things will.
  - jiggawatts 7y ago
    Here's the an important piece of real-world statistics for you, which I've read about in research papers and personally verified in large-scale systems: For every 1 complaint or help desk call you receive, there are between 100 and 1,000 users that experience the same issue but didn't bother to complain or call. Production testing is literally 0.1% to 1% as effective as actual testing.
    - toast0 7y ago
      Depending on your user numbers, this works out pretty well. If you've got 100k users, you're likely to get reports on anything that hits at least 1% of them. In my experience, you can write much simpler code that only needs to work in production than code that also needs to be unit testable. Adding hooks and separating bits and pieces so they can be tested can often make the code much less concise and harder to follow. I would rather spend engineering time making sure problem reports from users get read and escalated and addressed than on testing things that are better left untested. Unit tests, and even integration tests are unlikely to let you know when the feature works as designed, but not as users expect.
      - rob74 7y ago
        My favorite example of "things that are better left untested" so far is a method that returned a hard-coded array and a test which checked whether the method really did return that hard-coded array. So, if you wanted to add something to that array, you now had to add it in two places. Sure, that's an easy way to increase test coverage, but maybe you should focus on areas where unit testing makes more sense first?
      - oselhn 7y ago
        I prefer to fix bugs before they hit production. It is almost impossible to do that without tests. Also testable code is usually much more readable than untestable god objects, tightly coupled modules and spagetti code.
        odshoifsdhfs 7y ago
  - Aeolun 7y ago
    > It's always a lie? You're basically arguing that code with bad test coverage never works. It may work, or it may not work. Without the tests you won’t ever actually know. Maybe your customers are bouncing on bugs, and while you are earning enough, you could have been earning 3 times as much without those bounces.
- tarsinge 7y ago
  And then you have tested code with useless unit tests that cheat on metrics and defects and customer complaints very high too, because nobody is listening to customers either way. I regularly run untested code (when appropriate, it’s a trade off for simpler design with less abstractions sometimes) with happy customers.
arkh 7y ago
You don't want to test code. Coverage is mostly useless. You want to test functionalities: setup end-to-end tests. It will be hard, it will take time and it often is a solo endeavor for the first months. But when tested functionalities stop breaking for no reasons even when you refactor a lot of code, when "the project for which a dev put tests" starts being "the project we are confident to push to prod without breaking things" you should get some traction. And once you think all your functionalities are tested you can measure code coverage: what is not covered should be dead code you can remove.
- sverhagen 7y ago
  Pointing people only at end-to-end testing seems poor advice. There is value in a mix of all sorts of testing. I agree that testing functionality blows counting coverage by the line out of the water. But since a lot of code that implements algorithms, string manipulation, etc. can be tested just fine with unit tests, and since end-to-end tests are expensive, easily more brittle, require a lot more dependencies, it would be better to have a healthy mix of different types of tests.
  - shkkmo 7y ago
    The comments was specifically about the use case of getting a large refactor merged. If you are simply adding code or making simple changes, then unit tests can have advantages. It is when you are making a large refactor that you need end-to-end testing to verify that nothing was broken.
- adrianN 7y ago
  Coverage is very useful in determining whether your tests actually do what you think they're doing. It's not very useful as a metric to be optimized. Writing good end-to-end tests that really cover the functionality as specified is very difficult, doing so blindly without tool support is doomed to fail. An even better tool than measuring coverage is setting up a mutation testing framework and adding tests until all mutations are caught.
  - 7y ago
DrScump 7y ago
a rewrite... is still more of a risk than fixing what isn't broken. Defining "broken" is the problem. A long-extant program may be "broken" even while it meets customer expectations, and "fixing" it could be viewed as a regression by the customer. I had a case early in my career where I was finishing up a rewrite of a report writer program. The old version didn't really have test data, so "testing" mainly meant comparing outputs of the legacy system vs. the rewrite. After I had sussed out all the errors, there were still some differences in which the legacy output just didn't make sense. I met with the customer liaison, and he checked with his users. It turned out that nobody had bothered even to glance at those specific reports in as long as anyone could remember.
- mcherm 7y ago
  So... did you correct the errors and release your updated (useless) report? Or did you slog through 7 levels of management approval to get permission to get rid of it?
  - DrScump 7y ago
    No, I just had the customer rep send a note appeoving the new system with unwanted reports removed. (This was a mainframe shop, where excess CPU, I/O, and printing had meaningful costs.)
- hinkley 7y ago
  Just because it works doesn’t mean it isn’t broken.
  - steverb 7y ago
    And just because a developer thinks it's broken doesn't mean it doesn't work exactly the way the customer likes it.
    - hinkley 7y ago
      Software is done when nobody is willing to work on it anymore. There are some things you do for employee retention in every industry. Typically the bits we are talking about are the bits the customer doesn't know about. Or they're caught in a cognitive dissonance loop where they want two things like features and stability but they won't agree that the two affect each other. So they ask for one until problems arise and then they pretend like we should have been honoring the other the whole time. Even when they told us it wasn't a priority. It's basic maintenance philosophy. It's not a priority to get your car to the shop. But it is a priority that it work reliably for your Thanksgiving trip in two weeks. Many things worth doing are inconvenient. As are many critical things.
cjfd 7y ago
It can be defended when the system is in a rather passive form of maintenance. E.g., every month a small bugfix. If it is more than that it becomes way to easy to fix one small thing while breaking three. Especially if the functionality is a little or a lot complex. What it does sound like, though, is that everything was refactored/covered in tests in one fell swoop. I find that questionable. I think it is much better to start out small in an untested code base. Just cover the small piece you are working on now in tests and gradually extend.
- Aeolun 7y ago
  Some code is so mangled together that trying to start small is more of an investment than overhauling the whole thing altogether.
raverbashing 7y ago
40% code coverage is meaningless It could be 40% weak unit tests that just go through the code but doesn't test much I would be happier if it were 40% coverage running integration tests with actual data and use cases
JohnBooty 7y ago
My thoughts exactly. I'm not merging a bunch of untested code that replaces a bunch of working code, no matter how bad the old code was! That seems like a recipe for a production nightmare. The fact that this situation even arose in the first place would be bad news. 1. Did the other dev just refactor the code on a Red Bull bender one night on his own time? Hopefully this refactor work is something we would have discussed ahead of time. Hopefully I wouldn't have to even tell them that the expected coverage for a big refactor like this would be darn close to 100%. 2. I'd phrase this rejection in a positive way. I'd be excited about the refactor and testability and I'd let the other dev know that I appreciated the work and was looking forward to getting it to 100%, testing and merging it (I realize the example in the story was purely hypothetical and this didn't actually happen. :P)
hinkley 7y ago
We may have diluted the meaning of refactor if we are arguing about pull requests. Refactoring is an activity of small steps of a particular kind that are generally sound transformations. The biggest risk is transcription errors. Of course one of the downsides of PRs is that the they muddle a sequence of individual commits and only show the cumulative result. When dealing with individuals with these sorts of fears, you have to be patient and strategic. Pick the most powerful refactor out of a set of many useful ones and do them one at a time until you achieve the result. Essentially you have to boil the frog.
- thfuran 7y ago
  >Refactoring is an activity of small steps of a particular kind that are generally sound transformations. The biggest risk is transcription errors. I think that is far from an established sole meaning of the term.
  - hinkley 7y ago
    And then I will repeat > We may have diluted the meaning of refactor And I will quote the summary for Refactoring by Martin Fowler: > Refactoring is a controlled technique for improving the design of an existing code base. Its essence is applying a series of small behavior-preserving transformations, each of which "too small to be worth doing"... If words don't mean anything it's really hard to have conversations and it's insanity to try to create strategy around them.
slowhand09 7y ago
That security hole that is leaking customer data hasn't been noticed for the 9 months since it was implemented. Except by that one guy who has captured it all. Customers have only vetted what they see.

AstralStorm 7y ago

It's always a lie, about running in production. I've personally seen untested code allegedly running but it isn't actually working. Defect numbers through the roof and test harness literally not being merged exactly because of this issue. You will get visibility, either by customer complaints, slowing down development speed and ultimately projects being shut down. It's not sabotage and it's not exactly drowning. It's willful attempt to ignore reality and cheat on metrics, prolonging the inevitable and defrauding stakeholders.

BigJono 7y ago
It's always a lie? You're basically arguing that code with bad test coverage never works. Production code is copping constant manual end-to-end tests. Those tests are more valuable than most unit testing, and some automated black box testing. I'll take stable production system over the unit tested new code any day of the week.
- peterkelly 7y ago
  That's the kind of thinking that leads to situations like this: https://www.digitaltrends.com/mobile/samsung-galaxy-s10-ultrasonic-fingerprint-scanner-fooled-by-screen-protector/ https://www.digitaltrends.com/mobile/samsung-galaxy-s10-ultr... A lot of the time it's just as important to make sure certain things can't work as making sure that certain other things will.
- jiggawatts 7y ago
  Here's the an important piece of real-world statistics for you, which I've read about in research papers and personally verified in large-scale systems: For every 1 complaint or help desk call you receive, there are between 100 and 1,000 users that experience the same issue but didn't bother to complain or call. Production testing is literally 0.1% to 1% as effective as actual testing.
  - toast0 7y ago
    Depending on your user numbers, this works out pretty well. If you've got 100k users, you're likely to get reports on anything that hits at least 1% of them. In my experience, you can write much simpler code that only needs to work in production than code that also needs to be unit testable. Adding hooks and separating bits and pieces so they can be tested can often make the code much less concise and harder to follow. I would rather spend engineering time making sure problem reports from users get read and escalated and addressed than on testing things that are better left untested. Unit tests, and even integration tests are unlikely to let you know when the feature works as designed, but not as users expect.
    - rob74 7y ago
      My favorite example of "things that are better left untested" so far is a method that returned a hard-coded array and a test which checked whether the method really did return that hard-coded array. So, if you wanted to add something to that array, you now had to add it in two places. Sure, that's an easy way to increase test coverage, but maybe you should focus on areas where unit testing makes more sense first?
    - oselhn 7y ago
      I prefer to fix bugs before they hit production. It is almost impossible to do that without tests. Also testable code is usually much more readable than untestable god objects, tightly coupled modules and spagetti code.
      - odshoifsdhfs 7y ago
- Aeolun 7y ago
  > It's always a lie? You're basically arguing that code with bad test coverage never works. It may work, or it may not work. Without the tests you won’t ever actually know. Maybe your customers are bouncing on bugs, and while you are earning enough, you could have been earning 3 times as much without those bounces.
tarsinge 7y ago
And then you have tested code with useless unit tests that cheat on metrics and defects and customer complaints very high too, because nobody is listening to customers either way. I regularly run untested code (when appropriate, it’s a trade off for simpler design with less abstractions sometimes) with happy customers.

arkh 7y ago

You don't want to test code. Coverage is mostly useless. You want to test functionalities: setup end-to-end tests. It will be hard, it will take time and it often is a solo endeavor for the first months. But when tested functionalities stop breaking for no reasons even when you refactor a lot of code, when "the project for which a dev put tests" starts being "the project we are confident to push to prod without breaking things" you should get some traction. And once you think all your functionalities are tested you can measure code coverage: what is not covered should be dead code you can remove.

sverhagen 7y ago
Pointing people only at end-to-end testing seems poor advice. There is value in a mix of all sorts of testing. I agree that testing functionality blows counting coverage by the line out of the water. But since a lot of code that implements algorithms, string manipulation, etc. can be tested just fine with unit tests, and since end-to-end tests are expensive, easily more brittle, require a lot more dependencies, it would be better to have a healthy mix of different types of tests.
- shkkmo 7y ago
  The comments was specifically about the use case of getting a large refactor merged. If you are simply adding code or making simple changes, then unit tests can have advantages. It is when you are making a large refactor that you need end-to-end testing to verify that nothing was broken.
adrianN 7y ago
Coverage is very useful in determining whether your tests actually do what you think they're doing. It's not very useful as a metric to be optimized. Writing good end-to-end tests that really cover the functionality as specified is very difficult, doing so blindly without tool support is doomed to fail. An even better tool than measuring coverage is setting up a mutation testing framework and adding tests until all mutations are caught.
- 7y ago

DrScump 7y ago

a rewrite... is still more of a risk than fixing what isn't broken. Defining "broken" is the problem. A long-extant program may be "broken" even while it meets customer expectations, and "fixing" it could be viewed as a regression by the customer. I had a case early in my career where I was finishing up a rewrite of a report writer program. The old version didn't really have test data, so "testing" mainly meant comparing outputs of the legacy system vs. the rewrite. After I had sussed out all the errors, there were still some differences in which the legacy output just didn't make sense. I met with the customer liaison, and he checked with his users. It turned out that nobody had bothered even to glance at those specific reports in as long as anyone could remember.

mcherm 7y ago
So... did you correct the errors and release your updated (useless) report? Or did you slog through 7 levels of management approval to get permission to get rid of it?
- DrScump 7y ago
  No, I just had the customer rep send a note appeoving the new system with unwanted reports removed. (This was a mainframe shop, where excess CPU, I/O, and printing had meaningful costs.)
hinkley 7y ago
Just because it works doesn’t mean it isn’t broken.
- steverb 7y ago
  And just because a developer thinks it's broken doesn't mean it doesn't work exactly the way the customer likes it.
  - hinkley 7y ago
    Software is done when nobody is willing to work on it anymore. There are some things you do for employee retention in every industry. Typically the bits we are talking about are the bits the customer doesn't know about. Or they're caught in a cognitive dissonance loop where they want two things like features and stability but they won't agree that the two affect each other. So they ask for one until problems arise and then they pretend like we should have been honoring the other the whole time. Even when they told us it wasn't a priority. It's basic maintenance philosophy. It's not a priority to get your car to the shop. But it is a priority that it work reliably for your Thanksgiving trip in two weeks. Many things worth doing are inconvenient. As are many critical things.

cjfd 7y ago

It can be defended when the system is in a rather passive form of maintenance. E.g., every month a small bugfix. If it is more than that it becomes way to easy to fix one small thing while breaking three. Especially if the functionality is a little or a lot complex. What it does sound like, though, is that everything was refactored/covered in tests in one fell swoop. I find that questionable. I think it is much better to start out small in an untested code base. Just cover the small piece you are working on now in tests and gradually extend.

Aeolun 7y ago
Some code is so mangled together that trying to start small is more of an investment than overhauling the whole thing altogether.

raverbashing 7y ago

40% code coverage is meaningless It could be 40% weak unit tests that just go through the code but doesn't test much I would be happier if it were 40% coverage running integration tests with actual data and use cases

JohnBooty 7y ago

My thoughts exactly. I'm not merging a bunch of untested code that replaces a bunch of working code, no matter how bad the old code was! That seems like a recipe for a production nightmare. The fact that this situation even arose in the first place would be bad news. 1. Did the other dev just refactor the code on a Red Bull bender one night on his own time? Hopefully this refactor work is something we would have discussed ahead of time. Hopefully I wouldn't have to even tell them that the expected coverage for a big refactor like this would be darn close to 100%. 2. I'd phrase this rejection in a positive way. I'd be excited about the refactor and testability and I'd let the other dev know that I appreciated the work and was looking forward to getting it to 100%, testing and merging it (I realize the example in the story was purely hypothetical and this didn't actually happen. :P)

hinkley 7y ago

We may have diluted the meaning of refactor if we are arguing about pull requests. Refactoring is an activity of small steps of a particular kind that are generally sound transformations. The biggest risk is transcription errors. Of course one of the downsides of PRs is that the they muddle a sequence of individual commits and only show the cumulative result. When dealing with individuals with these sorts of fears, you have to be patient and strategic. Pick the most powerful refactor out of a set of many useful ones and do them one at a time until you achieve the result. Essentially you have to boil the frog.

thfuran 7y ago
>Refactoring is an activity of small steps of a particular kind that are generally sound transformations. The biggest risk is transcription errors. I think that is far from an established sole meaning of the term.
- hinkley 7y ago
  And then I will repeat > We may have diluted the meaning of refactor And I will quote the summary for Refactoring by Martin Fowler: > Refactoring is a controlled technique for improving the design of an existing code base. Its essence is applying a series of small behavior-preserving transformations, each of which "too small to be worth doing"... If words don't mean anything it's really hard to have conversations and it's insanity to try to create strategy around them.

slowhand09 7y ago

That security hole that is leaking customer data hasn't been noticed for the 9 months since it was implemented. Except by that one guy who has captured it all. Customers have only vetted what they see.