Remove two non-useful tests.

Review Request #1828 — Created Feb. 24, 2015 and submitted

benjyw
pants
34048c9...
pants-reviews
jsirois, patricklaw, zundel
All they were even claiming to exercise was the plumbing of a single config key.

But they weren't even doing that correctly in the new options world, and making
them do so would be pointless and brittle.

We should write proper tests for these tasks, that exercise the actual
logic. The deleted tests were just a burden.

Ran unit tests after the removal to be sure the BUILD file is OK.

ZU
  1. I agree these tests are pretty trivial, but they aren't completely useless. These tests did at least exercise the initialization code in the tasks and give a jumping off point for someone wanting to improve the tests.

    1. Well, the only thing they exercise is that a specific variable gets set to a specific value from a config file. That just doesn't seem worth the maintenance burden of these tests. For example, it will be a bit of work to get them to work with the new options system because they're currently so tightly bound to config. And once we do that they become even more trivial.

      If someone wants to write proper tests for these tasks they'd be better off starting from scratch, quite frankly. I prefer no test at all to a test that isn't worth its weight in lines of code, or that gives us a false sense of security about our coverage.

    2. Another argument is that - even if you're in favor of the good of the check, its outweighed by the bad of the cargo cult it will lead to.  Bad examples are probably x2 damage weapons.
    3. You could have just stripped the bad part out of the test is all I'm saying. It is surprising when I make a code change and all tests pass, only to go to find not the least bit of coverage for the module.

    4. Aha - this is an RB problem - it would be nice if it displayed the deleted files in the left hand pane.  These tests were comprised of 100% the bad parts.
    5. Ah yes, in case it wasn't clear, these tests were just stubs.

  2. 
      
PA
  1. Ship It!
  2. 
      
BE
  1. Submitted as 09421571665084c1f003d3fa2d9c3a59a9af204a.

  2. 
      
BE
Review request changed

Status: Closed (submitted)

JS
  1. Ship It!
  2. 
      
Loading...