6 ms·

I may be being stupid here, but shouldn't you really be using a comprehensive test suite and a code coverage tool like simplecov [0] to automatically show you y

I may be being stupid here, but shouldn't you really be using a comprehensive test suite and a code coverage tool like simplecov [0] to automatically show you your dead code? Or if you don't do test coverage (!) something like coverband [1] in production? That way you get to see straight away which code is genuinely dead rather than having to go spelunking with `git-bisect`? [0] https://github.com/colszowka/simplecov https://github.com/colszowka/simplecov [1] https://github.com/danmayer/coverband https://github.com/danmayer/coverband

gknoy 9y ago
I don't think it would work as well as we'd hope. Our projects aim for ever-increasing code coverage; it seems silly, esp as we get closer to 100%, but it seems to be pertinent here. If I were to refactor some function to no longer be used, one would hope/expect that code coverage for that function would be missing, and thus we'd realize (or at least be able to notice more easily) that it was dead code. However, unless your code is all tested via end-to-end tests of features and capabilities, it's _very_ possible that I (or a teammate) wrote some test of our dead helper function, verifying that it handles its edge cases correctly, as a unit test. In that case, we'd still see our code as "covered". I think we could work around that by making the function deliberately raise an exception, and see which tests break, or commenting out the unit test of that function and see if it's still used. If the code is being exercised in any way other than its own unit tests, it ought to raise an error (or show coverage, depending on tactic). However, this requires us to _already_ suspect that function is dead.
- chris_wot 9y ago
  If there was sufficient code coverage then the test would hopefully invoke a function that eventually calls on your potentially dead function.
  - eduren 9y ago
    I think the poster was saying that proper unit testing would give false positives. Example: If I have a "dead" add() function with a unit test that asserts add(2,2) == 4, then code coverage would report that function being covered even if no other part of the codebase uses it anymore.
    - gknoy 9y ago
      That's exactly what I was trying to say -- thanks! I still _want_ that kind of test coverage, esp when there's weird edge cases, but when looking for dead code, it's probably good to look at only the acceptance / integration tests (or whatever you want to call the tests that do the holistic "I click here and everything works" tests).
dom0 9y ago
You seem to assume that a comprehensive test suite exists...
falcolas 9y ago
100% coverage in testing doesn't indicate 100% usage in production. For example - if an API endpoint is no longer in use (say, it's a v1, and your company has now entirely moved to v4), the code in question will still be covered by testing - it's not dead according to the test suite.
- tcopeland 9y ago
  Yup but coverband doesn't do code coverage during tests (like simplecov); it does code coverage in production by sampling requests and profiling them.

gknoy 9y ago

I don't think it would work as well as we'd hope. Our projects aim for ever-increasing code coverage; it seems silly, esp as we get closer to 100%, but it seems to be pertinent here. If I were to refactor some function to no longer be used, one would hope/expect that code coverage for that function would be missing, and thus we'd realize (or at least be able to notice more easily) that it was dead code. However, unless your code is all tested via end-to-end tests of features and capabilities, it's _very_ possible that I (or a teammate) wrote some test of our dead helper function, verifying that it handles its edge cases correctly, as a unit test. In that case, we'd still see our code as "covered". I think we could work around that by making the function deliberately raise an exception, and see which tests break, or commenting out the unit test of that function and see if it's still used. If the code is being exercised in any way other than its own unit tests, it ought to raise an error (or show coverage, depending on tactic). However, this requires us to _already_ suspect that function is dead.

chris_wot 9y ago
If there was sufficient code coverage then the test would hopefully invoke a function that eventually calls on your potentially dead function.
- eduren 9y ago
  I think the poster was saying that proper unit testing would give false positives. Example: If I have a "dead" add() function with a unit test that asserts add(2,2) == 4, then code coverage would report that function being covered even if no other part of the codebase uses it anymore.
  - gknoy 9y ago
    That's exactly what I was trying to say -- thanks! I still _want_ that kind of test coverage, esp when there's weird edge cases, but when looking for dead code, it's probably good to look at only the acceptance / integration tests (or whatever you want to call the tests that do the holistic "I click here and everything works" tests).

dom0 9y ago

You seem to assume that a comprehensive test suite exists...

falcolas 9y ago

100% coverage in testing doesn't indicate 100% usage in production. For example - if an API endpoint is no longer in use (say, it's a v1, and your company has now entirely moved to v4), the code in question will still be covered by testing - it's not dead according to the test suite.

tcopeland 9y ago
Yup but coverband doesn't do code coverage during tests (like simplecov); it does code coverage in production by sampling requests and profiling them.