integration tests fail on Windows

Some integration tests on Windows fail, and here is a fix: https://github.com/beckbaibai/stem/tree/testwin

Trac:
Username: reganeet

added component::archived/stem owner::reganeet priority::medium reporter::reganeet resolution::fixed status::closed type::defect labels

Trac:
Status: new to needs_review

Looks great! Only a couple questions...

1. stem/util/system.py expand_path - Don't we need the drive letters for a path to be absolute? That was the bit that I suspected would be difficult, though if it's not necessary then great.

2. test/integ/util/system.py - Why are you skipping these tests if the pgrep lookup fails? It's just checking if there are multiple tor instances, which we could probably default to 'False'.

Cheers! -Damian

Trac:
Status: needs_review to needs_information

Don't we need the drive letters for a path to be absolute? 

Yes we do; os.getcwd() will provide an absolute path with the drive letter. Are you suggesting that when a path is provided, we should append it to the current drive letter and then check if it is absolute?

Why are you skipping these tests if the pgrep lookup fails?

Huh, I thought it is required for the test cases to run. I was wrong. Can we set the default value of "is_extra_tor_running" to False, get rid of "if self.is_extra_tor_running is None" in setUp(), and simply pass it on windows?

Beck

Trac:
Username: reganeet

Yes we do; os.getcwd() will provide an absolute path with the drive letter.

Ahh, gotcha.

Can we set the default value of "is_extra_tor_running" to False, get rid of "if self.is_extra_tor_running is None" in setUp()

Simplest is probably just to have an 'is_windows()' check and hardcode it to False in that case. It would be nice if we could later check this on Windows, but not at all vital.

Pushed your changes with some minor tweaks... https://gitweb.torproject.org/stem.git/commitdiff/6f626f3

Would you mind exercising these new Windows expand_path() use cases in the unit tests? If you look in 'test/unit/util/system.py' you'll find a 'test_expand_path_unix' test. It would be great to have a windows counterpart for this. :)

Trac:
Status: needs_information to needs_revision

Pushed your changes with some minor tweaks...

Some test cases in test/integ/util/system.py now fail. Do you want me to fix them?

It would be great to have a windows counterpart for this.

No problem.

Trac:
Username: reganeet

Some test cases in test/integ/util/system.py now fail. Do you want me to fix them?

Yes please. :)

Hi atagar:

In stem/util/system.py line 512, why did you add an is_windows() check? This makes the method fail to expand tildas on windows. os.path.expanduser(relative_path) works perfectly on windows:

>>> os.path.expanduser("~")
'C:\\Users\\Beck'

So imo, we should remove this is_windows() check.

Beck

Trac:
Username: reganeet

Oh, didn't realize that worked on windows. Feel free to put it back.

Fixed test cases on windows and added test_expand_path_windows: https://github.com/beckbaibai/stem/tree/testwin

Trac:
Username: reganeet

Fixes pushed. Thanks!

Trac:
Resolution: N/A to fixed
Status: needs_revision to closed

closed