[MERGE] add win32utils.get_local_appdata_location()

John Arbash Meinel john at arbash-meinel.com
Tue Aug 26 20:34:27 BST 2008


John Arbash Meinel has voted resubmit.
Status is now: Resubmit
Comment:
For environment variables, we have "osutils.set_or_unset_env". Though 
what you probably really want is TestCase._captureVar('VAR', 'NEWVALUE')

That will reset the env var at the beginning, and restore it during 
tearDown.

          self.assertEquals('not-existing', p)
+
+class TestLocationsCtypes(TestCase):
^- Two blank lines, please


You shouldn't do "from bzrlib import get_user_encoding()" as that 
function is imported from "osutils" you should just use 
"osutils.get_user_encoding()".

I would also request that you use

from bzrlib import win32utils

and then

win32utils.foo

rather than

bzrlib.win32utils.foo

One of the big wins is because of LazyImports. We've had problems in the 
past where someone was using "bzrlib.branch.XXX" and it "just worked" 
without importing, because it happened to be imported by some other 
module. And then we cleaned up that module and code starts breaking. It 
doesn't matter a lot for the test suite, but it is good to maintain the 
habit.

I also see you doing "os.environ[FOO].decode(user_encoding())" however, 
I thought you made the case that ENV vars are in MBCS encoding, not the 
current codepage. Certainly I would expect one or the other to be 
correct.

I'm also a bit concerned, as I've seen my share of "C:\Progra~1\User~1\" 
values coming back out of things like environ, and that wouldn't match 
the return value of a win32 api call.

+def _get_sh_special_folder_path(csidl):
+    """Call SHGetSpecialFolderPathW if available, or return None.
+
+    Result is always unicode (or None).
+    """
+    if has_ctypes:
+        try:
+            SHGetSpecialFolderPath = \
+                ctypes.windll.shell32.SHGetSpecialFolderPathW
+        except AttributeError:
+            pass

^- I don't know if this function was even present on Win98, but I know 
Alexander has a lot of code to allow either the "A" form or the "W" form 
based on the settings. You may want to use a similar pattern here.


As the tests look pretty much identical (aside from overriding 
has_ctypes), it seems like inheriting the other test, and providing a 
new setUp would be better than duplicating the tests verbatim. I believe 
the "preferred" method is to use load_all_tests() and provide an 
adapter. Though I still find that harder to do than just doing "class 
TestWin32Util(TestCtypes): "




For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C033001c904dd%24f04c5e30%24d0e51a90%24%40com.au%3E
Project: Bazaar



More information about the bazaar mailing list