Can bzr remember ftp-passwords?

[Martin Pool]

Chris posted an updated patch with tests, thanks very much.  I know
they can be annoying to write but it does pay off.  I have some
suggestions for the tests:

http://librarian.launchpad.net/7296677/bzrbundle1

-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import TestCaseWithTransport, TestSkipped, TestCaseInTempDir
 import bzrlib.transport
+import bzrlib.transport.ftp as ftp
+import tempfile
+import os

We normally put the imports from the standard library first, and
within each group they go in alphabetical order.

http://www.python.org/dev/peps/pep-0008/

+
+class TestNetrc(TestCaseInTempDir):
+    def test_netrc_dontexist(self):

And the style guide also says there should be one blank line after the
class name.

+    def test_named_tempfiles(self):
+        f = tempfile.NamedTemporaryFile()
+        name = f.name
+
+        f.write('hello')
+        f.flush()
+        g = open(f.name)
+        self.assertEqual(g.name, f.name)
+
+

This seems to test that the NamedTemporaryFile works.  It is an
entirely reasonable thing to do while you're developing to make sure
you understand the api but not the kind we need to keep in the long
term.

+    def test_netrc_dontexist(self):
+        """ Test password retrieval if netrc-file does not exist. """
+        f = 'nonexistingfile'
+        if not os.path.exists(f):
+            # Hope it isn't being created suddenly.
+            check = ftp._find_stored_password
+            self.assertEqual(check('remotehost', 'broken1', file=f),
+                    None)
+            return
+        else:
+            raise TestSkipped('No unique filename found')

Since each test runs in its own directory it would be calamitous if
that file existed or was created.  you don't need to test for it.

But more importantly, rather than passing a special file path to
_find_stored_password, why not just write the file into
self.test_home_dir + '/.netrc', and then you should be able to find it
under the regular name?

+        f.flush()
+        f.seek(0,0)

You should do instead

  f = open()
  try:
    write
  finally:
    f.close()

Windows gets annoyed by files that aren't closed...

+        self.assertEqual(os.path.isfile(name), True)

again I think you can delete this once the test is working as you expect.

-- 
Martin