search

Albacore / albacore

Albacore is a professional quality suite of Rake tasks for building .NET or Mono based systems.
www.albacorebuild.net
221 stars 64 forks source link

Support batching tests #212

Closed blairconrad closed 8 years ago

blairconrad commented 8 years ago

Fixes #207.

@haf, I have to ask forgiveness. I have broken the cardinal rule of Pull Requests by substantially changing the base code, and its style. I fear it will not be to your taste, and apologize. But if you have patience to work with me, I'm happy to fix anything you don't like.

An explanation:

In order to test the test-batching, I found that I would need access to the TestRunner::Cmd objects that the TestRunner::Task was making. Relying on creating a Cmd in the specs and investigating its behaviour would no longer work. So I made a change to the Task to support this. At the same time, it became clear that poking execute_tests_for dll wouldn't work either. So I, in stages, replaced most of the specs so they would be driven from the Task level, and cover (nearly) all behaviour in the Task and Cmds. It's counter to the existing approach, but is more refactoring-friendly, and is an approach that I've seen work very well in other projects. By driving the code in a way that more closely matches how the users would, you can worry a little less about the internal details.

So:

  1. I introduced a private execute_commands method on the Task, which is overridden in the specs. The original behaviour is so simple that not exercising it during the specs is hopefully forgivable. I'm not usually a fan of overriding production methods in tests, but it seems no worse than sending a signal to a private method.
  2. I updated existing specs to provide close to end-to-end coverage, from the config object down to the command, to mirror how the new specs would be written.
  3. I added minor extra specs, e.g. for is_ms_test. I did not add an end-to-end spec for copy_local, as I wasn't sure you'd want to add all that extra copying around during test runs
  4. The new specs change their working directory to better work with existing "test" files. The task requires test files to actually exist, so I co-opted existing files in the hierarchy. By running in the specs directory, it's possible to test a negative relative path exactly. I know, perhaps this was an odd choice.
  5. Finally introduced specs for the execute_as_batch option, and implemented the feature. There's no "path shortening" feature for this yet - the tests would execute in the current directory. If you'd like, I can see about looking for the lowest common directory above the "test files" and execute from there, or some such. Or this could be introduced later, if you think it worthwhile.

So, yeah. Perhaps not what you were expecting. And maybe totally against the style you'd like to have for the tests. And my lack of ruby/rspec knowledge probably means I've made a terrible mess of all of it. Sorry. But again, if you're willing, I'll work to mold things into a shape you prefer.

Thanks for considering the PR.

blairconrad commented 8 years ago

For the love of Pete, I started the PR from the wrong branch, where I'd had work in progress for shortening paths. Will re-PR.