Bug 425510 & Bug 426410 (shell completion on win32)

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Nov 11 06:19:46 GMT 2009


Sorry for the delay :-/
>>>>> "Maritza" == Maritza Mendez <martitzam at gmail.com> writes:

<snip/>

    Maritza> I welcome feedback on everything, including especially

    Maritza> * how I set up my branch on lp

So, the way you set it up means you're the only one able to push there,
which is certainly what you wanted.

Alternatively you could have used lp:~bzr/bzr/whatever to share write
access with the whole bzr team, which you're welcome to join:
https://edge.launchpad.net/~bzr. 

    Maritza> * the location of the tests [2]

See below.

    Maritza> * the tests themselves

There is duplication between them, but since they need to end up in
different files, there is no point in trying to address that (more on
that below too).

    Maritza> And I offer the following observations:

    Maritza> * notice that the output of 'bzr rm' is written to stderr
    Maritza> instead of stdout.  Realizing hat is what took me the
    Maritza> longest time!

Hmm, so a trick I used while developing the feature and writing the
tests was to just put a random expected output and expected errors just
to have the test fail and display what was expected.

    Maritza> * since the order of output lines could theoreticaly change
    Maritza> in a future release of bzr without affecting the validity
    Maritza> of the tests, I was tempted to use '...' in the output
    Maritza> matching.  But I decided to go with the exact output, since
    Maritza> ... is too big a "hole" for these tests.  But thsi is
    Maritza> brittle wrt future releases which may change output order.

Then we'll update the tests. I'm pretty sure it's ok in that case as the
output order is deterministic anyway.

    Maritza> 
    Maritza> Ideally, we would have some regexp-by-line capability which
    Maritza> would say "the output must contain all of these lines but
    Maritza> in any order."  I know I could accomplish this by piping
    Maritza> the output of bzr to sort and grep, but that makes the
    Maritza> tests hard to read and besides I am lazy.  :)

    Maritza> Also, a huge thank you to everyone who is providing
    Maritza> guidance to me on these bugs, especially Vincent for the
    Maritza> shell-like tests.

    Maritza> Next step: get feedback and then implement the (trivial)
    Maritza> fixes for these bugs and repeat tests on karmic and win32.

Sorry again for the late feedback, but hopefully it's still useful.


    Maritza> ~M

    Maritza> [1] I was able to get up and running with just a few hints
    Maritza> from Vincent.  It literally took me less than an hour to
    Maritza> create two tests, and this was my first time creating *any*
    Maritza> test for bzr.

That's great to hear, thanks a lot for saying it as this is exactly why
I implemented them: any user of bzr should be able to do the same
without having to learn *where* to put these tests nor using more
advanced techniques to write them in a more "optimal" way. And I think
that even if your code itself doesn't end up in bzr, you helped a lot in
fixing the bugs by providing the tests since, from there, it was far
easier to diagnose the problem.

    Maritza> [2] I have tentatively left the tests among the unittests,
    Maritza> even though they *seem* like they belong with the blackbox
    Maritza> tests.

You're totally right.  So you wanted test_mv.py and test_remove.py, the
class you defined was perfect otherwise.

    Maritza> I did this because the bb tests seem to be command-specific
    Maritza> (as strongly suggested in doc/developers/testing.txt) and I
    Maritza> wanted to keep the wildcard matching tests all together in
    Maritza> one place for now.  I'm open to improvements, naturally.

Well, sometimes you can't satisfy all constraints at once :-) 

In that case you really wanted to do that as bb tests (easy to tell now
that you provided the fixes :-) since you are changing the way the
commands behave. There is just no other way to test your fixes.

         Vincent



More information about the bazaar mailing list