Should this code be rewritten or refactored

The old tests are just fine for verifying that calculateTax still works as it should. However, you don't need many test cases for this, only 3 (or maybe some more, if you want to test error handling too, using unexpected values of name).

Each of the individual cases (at the moment implemented in doSomething et al.) must have their own set of tests too, which test the inner details and special cases related to each implementation. In the new setup these tests could / should be converted into direct tests on the respective Strategy class.

I prefer to remove old unit tests only if the code they exercise, and the functionality it implements, completely ceases to exist. Otherwise, the knowledge encoded into these tests is still relevant, only the tests need to be refactored themselves.

Update

There may be some duplication between the tests of calculateTax (let's call them high level tests) and the tests for the individual calculation strategies (low level tests) - it depends on your implementation.

I guess the original implementation of your tests asserts the result of the specific tax calculation, implicitly verifying that the specific calculation strategy was used to produce it. If you keep this schema, you will have duplication indeed. However, as @Kristof hinted, you may implement the high level tests using mocks too, to verify only that the right kind of (mock) strategy was selected and invoked by calculateTax. In this case there will be no duplication between high and low level tests.

So if refactoring the affected tests isn't too costly, I would prefer the latter approach. However, in real life, when doing some massive refactoring, I do tolerate a small amount of test code duplication if it saves me enough time :-)

Should I test the Factory class too?

Again, it depends. Note that the tests of calculateTax effectively test the factory. So if the factory code is a trivial switch block like your code above, these tests may be all you need. But if the factory does some more tricky things, you may want to dedicate some tests specifically for it. It all boils down to how much tests you need to be confident that the code in question really works. If, upon reading the code - or analysing code coverage data - you see untested execution paths, dedicate some more tests to exercise these. Then repeat this until you are fully confident in your code.

OK, so there are now more things here than are suitable for a comment.

tl;dr

Your intuition about what you ought to do (refactoring as you go) is correct.

Your difficulty implementing this - given that you have to work around a poor code review system - comes down to your difficulty manipulating your source code and VCS. Multiple people have said that you can and should split your changes (yes, even within files) into multiple commits, but you seem to have difficulty believing this.

You really can do this. That really is what we're suggesting. You really should learn to get the most out of your editing, source manipulation and version control tools. If you invest the time in learning how to use them well, it makes life much easier.

Workflow/office politics issues

I'd make essentially the same recommendation as GlenH7 that you create two commits - one with only refactorings and (demonstrably or obviously) no functional changes, and one with the functional changes in question.

It may be useful though, if you're finding lots of errors, to pick a single category of error to fix within a single CR. Then you have one commit with a comment like "dedupe code", "fix type-X errors", or whatever. Because this makes a single type of change, presumably in multiple places, it should be trivial to review. It does mean you can't fix every error you find, but may make it less painful to smuggle through.

VCS issues

Splitting the changes made to your working source into multiple commits shouldn't be a challenge. You haven't said what you're using, but possible workflows are:

1. if you're using git, you have excellent tools for this

   • you can use git add -i for interactive staging from the command line
   • you can use git gui and select individual hunks and lines to stage (this is one of the few VCS related GUI tools I actually prefer to the command line, the other being a good 3-way merge editor)
   • you can make lots of tiny commits (individual changes, or fixing the same class of bug in multiple places) and then reorder them, merge them or split them with rebase -i

2. if you're not using git, your VCS may still have tooling for this sort of thing, but I can't advise without knowing what system you use

   You mentioned you're using TFS - which I believe is compatible with git since TFS2013. It may be worth experimenting with using a local git clone of the repo to work in. If this is disabled or doesn't work for you, you can still import the source into a local git repo, work in there, and use it to export your final commits.

3. you can do it manually in any VCS if you have access to basic tools like diff and patch. It's more painful, but certainly possible. The basic workflow would be:

   1. make all your changes, test, etc.
   2. use diff to make a (context or unified) patch file with all changes since the last commit
   3. partition that into two files: you'll end up with one patch file containing refactorings, and one with functional changes
      • this isn't entirely trivial - tools such as emacs diff mode may help
   4. back everything up
   5. revert to the last commit, use patch to replay one of the patch files, commit those changes
      • NB. if the patch didn't apply cleanly, you may need to fix up the failed hunks manually
   6. repeat 5 for the second patch file

Now you have two commits, with your changes partitioned appropriately.

Note there are probably tools to make this patch management easier - I just don't use them since git does it for me.