-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Vincent Ladeuil wrote: > Vincent Ladeuil has proposed merging lp:~vila/bzr/shell-like-tests into lp:bzr. > > Requested reviews: > bzr-core (bzr-core) > > This implement a new TestCase that provides the ability to run shell-like scenarios like: > > class TestConflict(script.TestCaseWithScript): > > def test_content_conflict(self): > story = """ > bzr init trunk > cd trunk > echo "trunk content" >file > bzr add file > bzr commit -m 'Create trunk' > bzr branch . ../branch > cd ../branch > bzr rm file > bzr commit -m 'Delete file' > cd ../trunk > echo "more content" >>file > bzr commit -m 'Modify file' > cd ../branch > bzr merge ../trunk > """ > self.run_script(story) > wt, relpath = workingtree.WorkingTree.open_containing('branch') > self.assertEqual(1, len(wt.conflicts())) > > Commands implemented: bzr (ha ha), cat, echo, cd, mkdir. > > cd and mkdir are jailed. > Redirections are handled for echo and cat (<, >, >>). > > More can be done but I think that implementation is already usable and I don't want to > add too much features without getting some feedback first. review: needs_information 1) My first concern is that the above doesn't provide a way to do any assertions. It just provides a way to run a bunch of commands. If you look at how doctest does it, it does: >>> command_to_run() output We could do something similar with: $ command_to_run() output Though we then don't have a way to declare a different between stdout and stderr. Nor do we have a way to say that this command should succeed cleanly, or this command should fail in a particular way. Because of that, it means that the shell scripting really only works as a way to *set up* a test case. Which is arguably where we should be spending the least amount of time, not the bulk of the time as we work in 'command' level rather than internal object level. Maybe the overhead isn't as big as it once was, because we don't spawn anymore. But given Robert's recent threads about wanting to *speed up* selftest, this doesn't seem to be going in the right direction. 2) +class TestCaseWithScript(tests.TestCaseWithTransport): ... + def do_bzr(self, input, args): ... + def do_cat(self, input, args): ... It feels to me like things such as "do_bzr" should be part of a helper class, not part of the TestCase itself. So something more like: class ScriptClass(object): def __init__(self, actions_str): # parse and build up a list of actions def run(self): ... def do_bzr(...) def do_cat(...) def do_echo(...) class TestCaseWithScript(tests.TestCaseWithTransport): _script_class = ScriptClass def run_script(self, actions): script = self._script_class(actions) script.run() # Should we pass the TestCase (self) in? By making _script_class a member variable, test cases that want to override behavior can do so by subclassing ScriptClass rather than directly overriding self.do_foo() 3) I think the functionality is well tested. My biggest concerns are how is this actually going to benefit us. John =:-> -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqdTioACgkQJdeBCYSNAAOtggCfTZrOoCUzavBOTCynkIr23KEA UCUAnRcC0OSxvhMfOXfZD5M8kjHRkLzi =Lzrr -----END PGP SIGNATURE-----