Opened 3 years ago

Closed 3 years ago

#14844 closed enhancement (implemented)

Added Windows Support for Connection Resolution

Reported by: icodemachine Owned by: atagar
Priority: High Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: utils
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I have added Windows Support for Connection Resolution by making using of 'netstat -ano' command in Windows. It is working perfectly well on my end.

https://github.com/icodemachine/Stem/

Child Tickets

Change History (5)

comment:1 Changed 3 years ago by atagar

Hi icodemachine, this is a fantastic addition - thanks! For connection resolution it's important that we have tests, since otherwise I'll be unable to maintain it on my platform. Please see...

https://gitweb.torproject.org/stem.git/tree/test/unit/util/connection.py
https://gitweb.torproject.org/stem.git/tree/test/integ/util/connection.py

comment:2 Changed 3 years ago by icodemachine

Updated. Please check my repo under 'windows' branch.

https://github.com/icodemachine/Stem

comment:3 Changed 3 years ago by atagar

Very nice! Patch looks good and I'm tempted to merge, but there's a few issues at present...

  1. This doesn't pass stylistic checks since you're using tabs. Please install pyflakes and pep8 and you should see the issues...
* /home/atagar/Desktop/stem/test/integ/util/system.py
  line 218  - W293 W293 blank line contains whitespace
  line 218  - E101 E101 indentation contains mixed spaces and tabs
  line 218  - W191 W191 indentation contains tabs
  line 218  - indentation has a tab
  line 219  - E101 E101 indentation contains mixed spaces and tabs
  line 224  - local variable 'runner' is assigned to but never used
  line 239  - E303 E303 too many blank lines (2)

* /home/atagar/Desktop/stem/test/integ/util/connection.py
  line 43   - line has trailing whitespace
  line 67   - indentation has a tab
  line 67   - E101 E101 indentation contains mixed spaces and tabs
  line 67   - W191 W191 indentation contains tabs
  line 68   - E101 E101 indentation contains mixed spaces and tabs
  line 92   - W291 W291 trailing whitespace

... etc...
  1. In the unit tests you aren't mocking the new tasklist calls so the tests cause inadvertent system calls on windows and fail elsewhere...
======================================================================
ERROR: test_pid_by_name_lsof
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/home/atagar/Desktop/stem/test/unit/util/system.py", line 259, in test_pid_by_name_lsof
    self.assertEqual(expected_response, system.pid_by_name(test_input))
  File "/home/atagar/Desktop/stem/stem/util/system.py", line 458, in pid_by_name
    raise IOError("No results found for tasklist")
IOError: No results found for tasklist
  1. Integ tests fail for me with the following. Your addition should probably be wrapped with an 'if windows' check...
======================================================================
ERROR: test_pid_by_name
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/integ/util/system.py", line 89, in test_pid_by_name
    self.assertEqual(None, stem.util.system.pid_by_name('blarg_and_stuff'))
  File "/home/atagar/Desktop/stem/stem/util/system.py", line 477, in pid_by_name
    raise IOError("Unable to query '%s': %s" % (command, exc))
IOError: Unable to query 'tasklist | findstr blarg_and_stuff.exe.exe': [Errno 2] No such file or directory
  1. Some of your changes to system.py look to be breaking our python3 compatibility. Please install python3 and ensure the tests still work under that too.

comment:4 Changed 3 years ago by atagar

Keywords: utils added

Looks like we already had a ticket for this. Resolving #9850 as a duplicate of this.

comment:5 Changed 3 years ago by atagar

Resolution: implemented
Status: newclosed
Note: See TracTickets for help on using tickets.