Opened 5 years ago

Closed 5 years ago

#14076 closed defect (fixed)

Fix tutorial_examples.py to not relay on correct ordering.

Reported by: Foxboron Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: python, tests, tutorial, examples, 2, 3
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

https://github.com/Foxboron/stem/commit/93cfd8e31c99a2c489a9f1903bdda487c4444abb

The tests inside tutorial_examples.py relies on the correct order, as well as overriding sys.stdin. This works fine on python2, but breaks on python3 for reasons i haven't managed to track down.

However, this can be solved easily by just returning a list instead of overriding sys.stdin. This makes it easier to debug, and the tests does not have to rely on outputting the correct order.

This isn't ready to be merged yet, as assertListEqual got a new name on Python3, and i haven't found a neat way of overriding based on version yet.

Child Tickets

Change History (10)

comment:1 Changed 5 years ago by atagar

Gotcha. We should definitely fix any ordering dependency though have you thought of doing something like the following instead?

def assert_equal_unordered(self, expected, actual):
  self.assertListEqual(expected.splitlines(), actual.splitlines())

self.assert_equal_unordered(LIST_CIRCUITS_OUTPUT, stdout_output)

If we can't mock stdout that's a little unfortunate since I was thinking about changing these tests to...

def test_foo():
  ... do necessary mocking...

  with open('docs/tutorials/examples/foo') as docs_file:
    ... get the example code by looking for the '::' block

  exec(example)

  ... do assertions...

This way when examples are updated the tests will reflect that (rather than being duplicate code). This makes me a tad nervous cuz... well, exec(). But this might be a good use for it.

Removing our stdout mocks will take us a step away from being able to do this.

comment:2 Changed 5 years ago by Foxboron

Y'know, i did. But that was after the initial work was done You decision really, i think overriding sys.stoud is a very ugly, but if you got a good use for it later i can work around this.
Still need to figure out how to use the correct assertCountEqual vs assertListEqual depending on the Python version.

comment:3 Changed 5 years ago by atagar

Still need to figure out how to use the correct assertCountEqual vs assertListEqual depending on the Python version.

If it's just a matter of python versions then maybe the following?

import stem.prereq

def assert_equal_unordered(self, expected, actual):
  if stem.prereq.is_python_3():
    self.assertCountEqual(expected.splitlines(), actual.splitlines())
  else:
    self.assertListEqual(expected.splitlines(), actual.splitlines())

comment:4 Changed 5 years ago by Foxboron

That should work.
Then the question is if you want to override sys.stdin or not :)

comment:5 Changed 5 years ago by atagar

Tests presently pass for me with both python2 and python3, so the sys.stdin mocking is just a problem for #14075, right? If so then lets leave it out here and just have the patch be for ordering.

comment:6 Changed 5 years ago by Foxboron

Hah, i see i have affected you with my horrible mixing between stdin/stdout :D Totes my fault.

sys.stdout overriding was never a problem, it was just annoying to debug with. Thought i could also "fix" that while i was at it. But i can do as suggested and leave sys.stdout as-is and split the string up for comparison. I'll do that tomorrow \o/

comment:7 Changed 5 years ago by atagar

Hi Foxboron. Is this still an issue or did the issue go away when we merged #14075?

comment:8 Changed 5 years ago by Foxboron

Still an issue as the ordering gets messed up on 3.3+ for some strange reason. Patching this up today and adding the sys.stdout override back into the code.

comment:9 Changed 5 years ago by Foxboron

And the problem is solved with the branch!
https://github.com/Foxboron/stem/commit/2a3f29fe36f16fdd097682005182b13c1ce64c3f

Small commit in the end :)

Should be noted that with this patch, stem is passing all the current tests on everything between 2.6-3.4, and thats pretty damn neat!

Last edited 5 years ago by Foxboron (previous) (diff)

comment:10 Changed 5 years ago by atagar

Resolution: fixed
Status: newclosed

Thanks! Pushed.

Note: See TracTickets for help on using tickets.