Return named_tuple from WebSocketResponse.can_prepare #1020

esaezgil · 2016-07-29T21:13:20Z

What do these changes do?

Return named tuple from WebSocketResponse.can_prepare

Are there changes in behavior for the user?

No

Related issue number

#1016

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes

Update docs

coveralls · 2016-07-29T21:16:04Z

Coverage increased (+0.0006%) to 97.942% when pulling 8236bd9 on esaezgil:master into e2386a7 on KeepSafe:master.

coveralls · 2016-07-29T21:25:02Z

Coverage increased (+0.0006%) to 97.942% when pulling 8236bd9 on esaezgil:master into e2386a7 on KeepSafe:master.

coveralls · 2016-07-29T21:33:43Z

Coverage increased (+0.0006%) to 97.942% when pulling 8236bd9 on esaezgil:master into e2386a7 on KeepSafe:master.

coveralls · 2016-07-29T22:19:49Z

Coverage decreased (-2.2%) to 95.701% when pulling 868457e on esaezgil:master into e2386a7 on KeepSafe:master.

coveralls · 2016-07-29T22:38:59Z

Coverage increased (+0.0006%) to 97.942% when pulling 30d82f2 on esaezgil:master into e2386a7 on KeepSafe:master.

coveralls · 2016-07-29T23:19:19Z

Coverage increased (+0.0006%) to 97.942% when pulling 30d82f2 on esaezgil:master into e2386a7 on KeepSafe:master.

Update docs

coveralls · 2016-07-29T23:28:14Z

Coverage increased (+0.0006%) to 97.942% when pulling 9cceb5d on esaezgil:master into e2386a7 on KeepSafe:master.

asvetlov · 2016-07-30T10:03:48Z

tests/test_server.py

@@ -206,6 +206,7 @@ def test_srv_keep_alive(srv):
    assert not srv._keep_alive


+@pytest.mark.timeout(0)


Why did you modify non-related test?

~~This test timed out, (I guess because of the addition of pytest-timeout) and modified it to fix it~~
My bad

As I see tests are passed both locally and on travis-ci.
If timeout is an issue it should be fixed by separate PR anyway

asvetlov · 2016-07-30T10:04:49Z

Implement __bool__ method.
Write tests

Remove wrong test decorator

coveralls · 2016-07-30T15:58:14Z

Coverage increased (+0.0006%) to 97.928% when pulling 187c85a on esaezgil:master into 1a61823 on KeepSafe:master.

asvetlov · 2016-07-30T16:17:29Z

aiohttp/web_ws.py

@@ -37,6 +38,8 @@ def __init__(self, *,
        self._timeout = timeout
        self._autoclose = autoclose
        self._autoping = autoping
+        self._web_socket_ready = namedtuple('web_socket_ready',


Sorry, you did't get my point.
Push
WebSocketReady = namedtuple(WebSocketReady', ...)` at module namespace.

coveralls · 2016-07-30T17:07:24Z

Coverage increased (+0.0006%) to 97.928% when pulling c154a76 on esaezgil:master into 1a61823 on KeepSafe:master.

asvetlov · 2016-07-31T08:45:44Z

Much better but tests and __bool__ are still required.

asvetlov · 2016-08-11T08:31:23Z

tests/test_web_websocket.py

@@ -178,25 +178,29 @@ def test_write_non_prepared():
 def test_can_prepare_ok(make_request):
    req = make_request('GET', '/', protocols=True)
    ws = WebSocketResponse(protocols=('chat',))
+    assert bool(ws.can_prepare(request=req)) is True


Don't change existing tests but create a new one.

You don't need to call .can_prepare() -- just create an instance of WebSocketReady and test it.
I expect about three or four new tests

Did you write unittests before?

New UTs

codecov-io · 2016-08-11T15:39:13Z

Current coverage is 97.64% (diff: 100%)

Merging #1020 into master will increase coverage by <.01%

@@             master      #1020   diff @@
==========================================
  Files            28         28          
  Lines          6397       6401     +4   
  Methods           0          0          
  Messages          0          0          
  Branches       1085       1085          
==========================================
+ Hits           6246       6250     +4   
  Misses           79         79          
  Partials         72         72

Powered by Codecov. Last update 3759c40...f1bcc8e

asvetlov · 2016-08-14T00:12:20Z

Please add a test for __bool__.
I believe it is the last missing point.

esaezgil · 2016-08-14T17:10:46Z

Hi Andrew, thanks for reviewing the PR. I updated it after your last comment. Please let me know if you have any other comments.
Enrique

asvetlov · 2016-08-25T13:00:03Z

Thanks!

lock · 2019-10-29T15:04:15Z

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

Return named_tuple from WebSocketResponse.can_prepare

8236bd9

Update docs

esaezgil closed this Jul 29, 2016

esaezgil reopened this Jul 29, 2016

esaezgil closed this Jul 29, 2016

esaezgil reopened this Jul 29, 2016

esaezgil force-pushed the master branch from 868457e to 30d82f2 Compare July 29, 2016 22:36

esaezgil closed this Jul 29, 2016

esaezgil reopened this Jul 29, 2016

Modify the timeout associated to a specific test

9cceb5d

esaezgil force-pushed the master branch from 30d82f2 to 9cceb5d Compare July 29, 2016 23:20

Return named_tuple from WebSocketResponse.can_prepare

bbb6571

Update docs

asvetlov reviewed Jul 30, 2016
View reviewed changes

esaezgil added 3 commits July 30, 2016 17:49

Rename namedtuple attributes

37c630f

Remove wrong test decorator

Merge branch 'master' of https://github.com/esaezgil/aiohttp

002a9a3

Merge branch 'master' of https://github.com/KeepSafe/aiohttp

187c85a

asvetlov reviewed Jul 30, 2016
View reviewed changes

WebSocketReady at module namespace

c154a76

Merge https://github.com/KeepSafe/aiohttp

2849732

Add __bool__ to allow easy checking

11f8abc

esaezgil force-pushed the master branch from 16839c4 to 11f8abc Compare August 10, 2016 20:27

asvetlov reviewed Aug 11, 2016
View reviewed changes

Create a class for WebSocketReady implementing __bool__

359ce03

New UTs

Merge branch 'master' of https://github.com/KeepSafe/aiohttp

e563add

esaezgil added 2 commits August 14, 2016 18:47

Merge branch 'master' of https://github.com/KeepSafe/aiohttp

36b3b57

Add Unit tests for __bool__

f1bcc8e

asvetlov merged commit 568c074 into aio-libs:master Aug 25, 2016

asvetlov mentioned this pull request Aug 25, 2016

Return named tuple from WebSocketResponse.can_prepare #1016

Closed

lock bot added the outdated label Oct 29, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return named_tuple from WebSocketResponse.can_prepare #1020

Return named_tuple from WebSocketResponse.can_prepare #1020

esaezgil commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

asvetlov Jul 30, 2016

esaezgil Jul 30, 2016 •

edited

Loading

asvetlov Jul 30, 2016

asvetlov commented Jul 30, 2016

coveralls commented Jul 30, 2016 •

edited

Loading

asvetlov Jul 30, 2016

coveralls commented Jul 30, 2016 •

edited

Loading

asvetlov commented Jul 31, 2016

asvetlov Aug 11, 2016

asvetlov Aug 11, 2016

codecov-io commented Aug 11, 2016 •

edited

Loading

asvetlov commented Aug 14, 2016

esaezgil commented Aug 14, 2016

asvetlov commented Aug 25, 2016

lock bot commented Oct 29, 2019

		@@ -206,6 +206,7 @@ def test_srv_keep_alive(srv):
		assert not srv._keep_alive


		@pytest.mark.timeout(0)

Return named_tuple from WebSocketResponse.can_prepare #1020

Return named_tuple from WebSocketResponse.can_prepare #1020

Conversation

esaezgil commented Jul 29, 2016 • edited Loading

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

coveralls commented Jul 29, 2016 • edited Loading

asvetlov Jul 30, 2016

Choose a reason for hiding this comment

esaezgil Jul 30, 2016 • edited Loading

Choose a reason for hiding this comment

asvetlov Jul 30, 2016

Choose a reason for hiding this comment

asvetlov commented Jul 30, 2016

coveralls commented Jul 30, 2016 • edited Loading

asvetlov Jul 30, 2016

Choose a reason for hiding this comment

coveralls commented Jul 30, 2016 • edited Loading

asvetlov commented Jul 31, 2016

asvetlov Aug 11, 2016

Choose a reason for hiding this comment

asvetlov Aug 11, 2016

Choose a reason for hiding this comment

codecov-io commented Aug 11, 2016 • edited Loading

Current coverage is 97.64% (diff: 100%)

asvetlov commented Aug 14, 2016

esaezgil commented Aug 14, 2016

asvetlov commented Aug 25, 2016

lock bot commented Oct 29, 2019

esaezgil commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

esaezgil Jul 30, 2016 •

edited

Loading

coveralls commented Jul 30, 2016 •

edited

Loading

coveralls commented Jul 30, 2016 •

edited

Loading

codecov-io commented Aug 11, 2016 •

edited

Loading