Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

write tests #19

Closed
srid opened this issue Mar 19, 2013 · 18 comments
Closed

write tests #19

srid opened this issue Mar 19, 2013 · 18 comments
Labels
Milestone

Comments

@srid
Copy link
Contributor

srid commented Mar 19, 2013

test_api.py only tests the type of the return values. we need tests to cover the behaviour of appdirs functions (i.e., return values) on each platform.

@eddyp
Copy link
Contributor

eddyp commented Mar 19, 2013

(moving idea from the closed issue to here)

It would be really nice to be able to test the correct behaviour and values returned for all the supported platforms on any arbitrary system where Python is available (e.g.: test all platforms by running the tests on a RiscOS system)

@mcepl: how difficult would it be to test the actual behaviour for all supported platforms on any arbitrary platform?
I suppose that must involve some mock objects or monkey patched objects to correctly give appdirs the right environment to assume native execution. Is that feasible in the current implementation?

@eddyp eddyp mentioned this issue Mar 19, 2013
@mcepl
Copy link
Contributor

mcepl commented Mar 19, 2013

I have completely misunderstood this question. Of course, you mean testing Windows behavior on Linux etc. (or in Travis-CI, which is Linux-only I believe). Then I guess we need to do some mocking. Which may be a challenge for older releases of python.

(original answer)
I guess we would need to add some os.name conditions, but with addition to os.path.expandpath I think we should make something. I hope we could survive even without mocking and monkey-patching (that's per definition non-Pythonuesque, isn’t it?). Something like

observed = appdirs.user_cache_dir('')
if os.name() == 'posix':
    expected = os.path.expanduser('~/.cache')

self.assertTrue(os.path.samefile(observed, expected))

See the post mentioned in the comment on #17 for an example of the Windows values.

@eddyp
Copy link
Contributor

eddyp commented Mar 19, 2013

Yes, something like that.

@eddyp
Copy link
Contributor

eddyp commented Mar 21, 2013

@mcepl: Did you have some time to think about my proposal?

I am thinking that even if older version might not handle mocking or monkey patching correctly, appdirs itself could work correctly.
Taking that into account we could disable the cross-platform tests on older versions, and only allow native testing on older versions.

What do you think?

(I am really curious about some model of functional tests so I can write some for the XDG standard part, according to your example.)

@mcepl
Copy link
Contributor

mcepl commented Mar 22, 2013

Let me think a bit aloud.

The first concern is that we DO NOT want to test code outside of our project. Therefore, and to make our code as simple as possible and especially as platform-independent as possible. I would prefer using stdlib/win32 methods like os.path.expanduser (which should work on Windows or any platform Python runs) to get a home directory rather than weird speleology I see in _get_win_folder_from_registry (although, I am not sure what exactly this code does, not having Windows at hand, so maybe it is necessary). If in any way possible to replace _get_win_folder_from_registry or other heavily platform dependent functions, I would do it.

Concerning mocking itself. Yes, we cannot avoid it, but I would keep it as little as possible. So, probably when testing behavior on Windows, we should just mock a standard function and then compare the results.

Something like

with patch('os.name', return_value='winnt'):
    self.assertEqual(appdirs.user_cache_dir(''), "---some-kind-of-Windows-stuff"

Does it make sense?

@eddyp
Copy link
Contributor

eddyp commented Mar 22, 2013

2013/3/22 Matěj Cepl notifications@github.com

Let me think a bit aloud.

The first concern is that we DO NOT want to test code outside of our
project. Therefore, and to make our code as simple as possible and
especially as platform-independent as possible. I would prefer using
stdlib/win32 methods like os.path.expanduser (which should work on Windows
or any platform Python runs) to get a home directory rather than weird
speleology I see in _get_win_folder_from_registry (although, I am not sure
what exactly this code does, not having Windows at hand, so maybe it is
necessary). If in any way possible to replace _get_win_folder_from_registry
or other heavily platform dependent functions, I would do it.

Those functions, AIUI, try to adapt to various situations often present on
Windows systems. They, in turn, rely on other standard Windows APIs/things
such as importing _winreg or using win32com to get Windows specific
information. Things such as "which is the Windows folder?" are usually
easily answered by searching the registry, but I am not sure how much do
Python standard libraries offer information based on those.

I agree with simplification, but from what I can see, those
get_win_folder* functions are useful and try to do the right thing:

if sys.platform == "win32":
try:
import win32com.shell
_get_win_folder = _get_win_folder_with_pywin32
except ImportError:
try:
import ctypes
_get_win_folder = _get_win_folder_with_ctypes
except ImportError:
_get_win_folder = _get_win_folder_from_registry

Concerning mocking itself. Yes, we cannot avoid it, but I would keep it as
little as possible. So, probably when testing behavior on Windows, we
should just mock a standard functionhttp://stackoverflow.com/questions/12621049/patching-a-function-with-mock-only-for-one-moduleand then compare the results.

Something like

with patch('os.name', return_value='winnt'):
self.assertEqual(appdirs.user_cache_dir(''), "---some-kind-of-Windows-stuff"

Does it make sense?

If that info from the standard Python API grabs the correct info from
Windows registry, or whatever other source. If not, we'll have to mock
win32com.shell, ctypes, or _winreg, which doesn't seem to be such a
difficult job.

@srid's input would be useful.

@mcepl
Copy link
Contributor

mcepl commented Mar 22, 2013

I am sorry, but being who I am (an engineer at Red Hat), I have no clue about the Windows situation at all. But we would need somebody to investigate what exactly functions like os.path.expanduser give on Windows. I have assumed that they do The RIght Thing™ (otherwise we would hear screaming and howling from the Windows Python world), but we probably need now to know what actually this thing is.

@eddyp
Copy link
Contributor

eddyp commented Mar 22, 2013

@mcepl, @srid: just tested this on Windows 7 on a system at work where Python 2.7.2 is installed, os.path.expanduser("~") returns:

C:\ Users[login]

But the problem is site_* APIs can't use expanduser for obvious reasons.

@mcepl
Copy link
Contributor

mcepl commented Mar 22, 2013

I officially give up on resolving any questions for the platform I have no access to. Sorry.

@eddyp
Copy link
Contributor

eddyp commented Mar 22, 2013

I officially give up on resolving any questions for the platform I have no
access to. Sorry.
That's understandable.
Let's focus then on an example of cross-testing the *nix behaviour on any
platform. That's probably more productive.

@eddyp
Copy link
Contributor

eddyp commented Apr 5, 2013

@mcepl: any idea on the cross testing with focus on Linux? I started something based on py.test in my private repo, but I don't know what should be the model for unittest test frame.

@eddyp
Copy link
Contributor

eddyp commented Apr 18, 2013

@srid: Taking into account:

  • the current code is superior to 1.2.0 in the Linux area
  • the original testing capabilities are matched by the current code,
  • having functional tests would be nice, but it seems nobody has the time to implement them
  • users already asked for the Linux fixes

Wouldn't it be a good idea to release 1.3.0 now without the new tests which don't exist?

For my project I am still using a Linux only appdirs-like small class that works for now, because I know for a fact the latest appdirs release is broken, and the adoption of appdirs in other projects targeting Linux will surely not happen while the broken 1.2.0 version is the advertised latest and greatest.

@srid
Copy link
Contributor Author

srid commented May 9, 2013

+1 for the 1.3.0 release, but i'm going to defer this (need to first look into how it will affect our internal tools).

@eddyp
Copy link
Contributor

eddyp commented May 9, 2013

@srid : Great. How much time do you expect it will take you to do the needed checks?

@eddyp
Copy link
Contributor

eddyp commented May 24, 2013

@srid: any news regarding the release of 1.3.0?

@eddyp
Copy link
Contributor

eddyp commented Jun 10, 2013

@srid: Any news? One month has passed since our last interaction, and I don't see any users benefiting from this delay.

@eddyp
Copy link
Contributor

eddyp commented Nov 10, 2013

@srid: Are you still working on this code? Have you tested the impact it has on your tools?

Could you please release 1.3.0 already? The code was ready for release 6 months ago, but it looks like your company is keeping the code hostage for unknown reasons. This is not how the open source community works.

@srid
Copy link
Contributor Author

srid commented Apr 21, 2014

@eddyp - 1.3.0 has been released today, https://pypi.python.org/pypi/appdirs/1.3.0 ... next time, don't be so annoying. thanks for your contributions.

@zoofood zoofood closed this as completed May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants