[ 
https://issues.apache.org/jira/browse/PROTON-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17265041#comment-17265041
 ] 

ASF GitHub Bot commented on PROTON-2320:
----------------------------------------

jiridanek commented on a change in pull request #285:
URL: https://github.com/apache/qpid-proton/pull/285#discussion_r557563852



##########
File path: python/tests/proton_tests/engine.py
##########
@@ -53,2803 +54,2822 @@ def bytearray(x):
 
 OUTPUT_SIZE = 10*1024
 
+
 class Test(common.Test):
 
-  def __init__(self, *args):
-    common.Test.__init__(self, *args)
-    self._wires = []
-
-  def connection(self):
-    c1 = Connection()
-    c2 = Connection()
-    t1 = Transport()
-    t1.bind(c1)
-    t2 = Transport()
-    t2.bind(c2)
-    self._wires.append((c1, t1, c2, t2))
-
-    mask1 = 0
-    mask2 = 0
-
-    for cat in ("TRACE_FRM", "TRACE_RAW"):
-      trc = os.environ.get("PN_%s" % cat)
-      if trc and trc.lower() in ("1", "2", "yes", "true"):
-        mask1 = mask1 | getattr(Transport, cat)
-      if trc == "2":
-        mask2 = mask2 | getattr(Transport, cat)
-    t1.trace(mask1)
-    t2.trace(mask2)
-
-    return c1, c2
-
-  def link(self, name, max_frame=None, idle_timeout=None):
-    c1, c2 = self.connection()
-    if max_frame:
-      c1.transport.max_frame_size = max_frame[0]
-      c2.transport.max_frame_size = max_frame[1]
-    if idle_timeout:
-      # idle_timeout in seconds expressed as float
-      c1.transport.idle_timeout = idle_timeout[0]
-      c2.transport.idle_timeout = idle_timeout[1]
-    c1.open()
-    c2.open()
-    ssn1 = c1.session()
-    ssn1.open()
-    self.pump()
-    ssn2 = c2.session_head(Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_ACTIVE)
-    ssn2.open()
-    self.pump()
-    snd = ssn1.sender(name)
-    rcv = ssn2.receiver(name)
-    return snd, rcv
-
-  def cleanup(self):
-    self._wires = []
-
-  def pump(self, buffer_size=OUTPUT_SIZE):
-    for c1, t1, c2, t2 in self._wires:
-      pump(t1, t2, buffer_size)
+    def __init__(self, *args):
+        common.Test.__init__(self, *args)
+        self._wires = []
+
+    def connection(self):
+        c1 = Connection()
+        c2 = Connection()
+        t1 = Transport()
+        t1.bind(c1)
+        t2 = Transport()
+        t2.bind(c2)
+        self._wires.append((c1, t1, c2, t2))
+
+        mask1 = 0
+        mask2 = 0
+
+        for cat in ("TRACE_FRM", "TRACE_RAW"):
+            trc = os.environ.get("PN_%s" % cat)
+            if trc and trc.lower() in ("1", "2", "yes", "true"):
+                mask1 = mask1 | getattr(Transport, cat)
+            if trc == "2":
+                mask2 = mask2 | getattr(Transport, cat)
+        t1.trace(mask1)
+        t2.trace(mask2)
+
+        return c1, c2
+
+    def link(self, name, max_frame=None, idle_timeout=None):
+        c1, c2 = self.connection()
+        if max_frame:
+            c1.transport.max_frame_size = max_frame[0]
+            c2.transport.max_frame_size = max_frame[1]
+        if idle_timeout:
+            # idle_timeout in seconds expressed as float
+            c1.transport.idle_timeout = idle_timeout[0]
+            c2.transport.idle_timeout = idle_timeout[1]
+        c1.open()
+        c2.open()
+        ssn1 = c1.session()
+        ssn1.open()
+        self.pump()
+        ssn2 = c2.session_head(Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_ACTIVE)
+        ssn2.open()
+        self.pump()
+        snd = ssn1.sender(name)
+        rcv = ssn2.receiver(name)
+        return snd, rcv
+
+    def cleanup(self):
+        self._wires = []
+
+    def pump(self, buffer_size=OUTPUT_SIZE):
+        for c1, t1, c2, t2 in self._wires:
+            pump(t1, t2, buffer_size)
+
 
 class ConnectionTest(Test):
 
-  def setUp(self):
-    gc.enable()
-    self.c1, self.c2 = self.connection()
+    def setUp(self):
+        gc.enable()
+        self.c1, self.c2 = self.connection()
+
+    def cleanup(self):
+        # release resources created by this class
+        super(ConnectionTest, self).cleanup()
+        self.c1 = None
+        self.c2 = None
+
+    def tearDown(self):
+        self.cleanup()
+        gc.collect()
+        assert not gc.garbage
+
+    def test_open_close(self):
+        assert self.c1.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
+        assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
+
+        self.c1.open()
+        self.pump()
+
+        assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_UNINIT
+        assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_ACTIVE
+
+        self.c2.open()
+        self.pump()
+
+        assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
+        assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
 
-  def cleanup(self):
-    # release resources created by this class
-    super(ConnectionTest, self).cleanup()
-    self.c1 = None
-    self.c2 = None
+        self.c1.close()
+        self.pump()
 
-  def tearDown(self):
-    self.cleanup()
-    gc.collect()
-    assert not gc.garbage
+        assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_ACTIVE
+        assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_CLOSED
 
-  def test_open_close(self):
-    assert self.c1.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
-    assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
+        self.c2.close()
+        self.pump()
 
-    self.c1.open()
-    self.pump()
+        assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
+        assert self.c2.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
 
-    assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_UNINIT
-    assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_ACTIVE
+    def test_simultaneous_open_close(self):
+        assert self.c1.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
+        assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
 
-    self.c2.open()
-    self.pump()
+        self.c1.open()
+        self.c2.open()
 
-    assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
-    assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
+        self.pump()
 
-    self.c1.close()
-    self.pump()
+        assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
+        assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
 
-    assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_ACTIVE
-    assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_CLOSED
+        self.c1.close()
+        self.c2.close()
 
-    self.c2.close()
-    self.pump()
+        self.pump()
 
-    assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
-    assert self.c2.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
+        assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
+        assert self.c2.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
 
-  def test_simultaneous_open_close(self):
-    assert self.c1.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
-    assert self.c2.state == Endpoint.LOCAL_UNINIT | Endpoint.REMOTE_UNINIT
+    def test_capabilities_array(self):
+        self.c1.offered_capabilities = Array(UNDESCRIBED, Data.SYMBOL,
+                                             symbol("O_one"),
+                                             symbol("O_two"),
+                                             symbol("O_three"))
 
-    self.c1.open()
-    self.c2.open()
+        self.c1.desired_capabilities = Array(UNDESCRIBED, Data.SYMBOL,
+                                             symbol("D_one"),
+                                             symbol("D_two"),
+                                             symbol("D_three"))
+        self.c1.open()
 
-    self.pump()
+        assert self.c2.remote_offered_capabilities is None
+        assert self.c2.remote_desired_capabilities is None
 
-    assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
-    assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
+        self.pump()
 
-    self.c1.close()
-    self.c2.close()
+        assert self.c2.remote_offered_capabilities == 
self.c1.offered_capabilities, \
+            (self.c2.remote_offered_capabilities, self.c1.offered_capabilities)
+        assert self.c2.remote_desired_capabilities == 
self.c1.desired_capabilities, \
+            (self.c2.remote_desired_capabilities, self.c1.desired_capabilities)
 
-    self.pump()
+    def test_capabilities_symbol_list(self):
+        self.c1.offered_capabilities = SymbolList(['O_one', 'O_two', 
symbol('O_three')])
+        self.c1.desired_capabilities = SymbolList([symbol('D_one'), 'D_two', 
'D_three'])
+        self.c1.open()
 
-    assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
-    assert self.c2.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_CLOSED
+        assert self.c2.remote_offered_capabilities is None
+        assert self.c2.remote_desired_capabilities is None
 
-  def test_capabilities_array(self):
-    self.c1.offered_capabilities = Array(UNDESCRIBED, Data.SYMBOL,
-                                         symbol("O_one"),
-                                         symbol("O_two"),
-                                         symbol("O_three"))
+        self.pump()
 
-    self.c1.desired_capabilities = Array(UNDESCRIBED, Data.SYMBOL,
-                                         symbol("D_one"),
-                                         symbol("D_two"),
-                                         symbol("D_three"))
-    self.c1.open()
+        assert self.c2.remote_offered_capabilities == 
self.c1.offered_capabilities, \
+            (self.c2.remote_offered_capabilities, self.c1.offered_capabilities)
+        assert self.c2.remote_desired_capabilities == 
self.c1.desired_capabilities, \
+            (self.c2.remote_desired_capabilities, self.c1.desired_capabilities)
 
-    assert self.c2.remote_offered_capabilities is None
-    assert self.c2.remote_desired_capabilities is None
+    def test_condition(self):
+        self.c1.open()
+        self.c2.open()
+        self.pump()
+        assert self.c1.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
+        assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_ACTIVE
 
-    self.pump()
+        cond = Condition("blah:bleh", "this is a description", {symbol("foo"): 
"bar"})
+        self.c1.condition = cond
+        self.c1.close()
+
+        self.pump()
+
+        assert self.c1.state == Endpoint.LOCAL_CLOSED | Endpoint.REMOTE_ACTIVE
+        assert self.c2.state == Endpoint.LOCAL_ACTIVE | Endpoint.REMOTE_CLOSED
+
+        rcond = self.c2.remote_condition
+        assert rcond == cond, (rcond, cond)
+
+    def test_properties(self, p1=PropertyDict(key=symbol("value")), p2=None):
+        self.c1.properties = p1
+        self.c2.properties = p2
+        self.c1.open()
+        self.c2.open()
+        self.pump()
+
+        assert self.c2.remote_properties == p1, (self.c2.remote_properties, p1)
+        assert self.c1.remote_properties == p2, (self.c1.remote_properties, p2)
+
+    # The proton implementation limits channel_max to 32767.
+    # If I set the application's limit lower than that, I should
+    # get my wish.  If I set it higher -- not.
+    def test_channel_max_low(self, value=1234):
+        self.c1.transport.channel_max = value
+        self.c1.open()
+        self.pump()
+        assert self.c1.transport.channel_max == value, 
(self.c1.transport.channel_max, value)
+
+    def test_channel_max_high(self, value=65535):
+        self.c1.transport.channel_max = value
+        self.c1.open()
+        self.pump()
+        assert self.c1.transport.channel_max == 32767, 
(self.c1.transport.channel_max, value)
+
+    def test_channel_max_raise_and_lower(self):
+        upper_limit = 32767
+
+        # It's OK to lower the max below upper_limit.
+        self.c1.transport.channel_max = 12345
+        assert self.c1.transport.channel_max == 12345
+
+        # But it won't let us raise the limit above PN_IMPL_CHANNEL_MAX.
+        self.c1.transport.channel_max = 65535
+        assert self.c1.transport.channel_max == upper_limit
+
+        # send the OPEN frame
+        self.c1.open()
+        self.pump()
+
+        # Now it's too late to make any change, because
+        # we have already sent the OPEN frame.
+        try:
+            self.c1.transport.channel_max = 666
+            assert False, "expected session exception"
+        except:
+            pass
+
+        assert self.c1.transport.channel_max == upper_limit
+
+    # TODO: Currently failing test - PROTON-1759 - skip
+

Review comment:
       unfortunate newline; I won't be redoing the PR because of this




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


> Apply autofixes to resolve some flake8 code formatting issues
> -------------------------------------------------------------
>
>                 Key: PROTON-2320
>                 URL: https://issues.apache.org/jira/browse/PROTON-2320
>             Project: Qpid Proton
>          Issue Type: Task
>          Components: python-binding
>    Affects Versions: proton-c-0.33.0
>            Reporter: Jiri Daněk
>            Assignee: Jiri Daněk
>            Priority: Major
>             Fix For: proton-c-0.34.0
>
>
> Python code in Proton does not follow PEP8. There are automated tools which 
> can reformat the code to be more compliant (fix indentation, add spaces 
> around operators, ...).
> {noformat}
> pip install autopep8
> for f in `find -name "*.py"`; do autopep8 --in-place $f; done
> {noformat}
> Autopep8 has several level of "aggressiveness". The least aggressive setting 
> only changes whitespace. At a more aggressive setting, autopep8 will also 
> rewrite some code constructs.
> My plan is to commit this in several stages. Avoid mixing manual changes and 
> automatically generated changes in a single commit. Push the whitespace 
> changes first and only then let autopep8 to be more creative; otherwise the 
> rewrites get drowned in the huge initial diff.
> I don't want to add flake8 to CI jobs just yet; I want to wait a few days 
> with that.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to