Martin Panter added the comment:

Mark: My understanding is on Windows, winsock file descriptors and C library 
file descriptors are different beasts; see 
<https://docs.python.org/3/library/socket.html#socket.socket.fileno>. Perhaps 
the test should call socket functions like socket.recv() on the FD rather than 
C library functions via os.fdopen().

Victor: The test in this bug has started failing again, very likely due to your 
revision 7bd4736195ce enabling a timeout on the HTTP request. I guess this 
causes the socket to be in non-blocking mode, and read() to return None. This 
is what Issue 10119 tried to fix. Example:

http://buildbot.python.org/all/builders/AMD64%20Debian%20root%203.x/builds/3404/steps/test/logs/stdio
======================================================================
FAIL: test_fileno (test.test_urllibnet.urlopenNetworkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File 
"/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_urllibnet.py", 
line 110, in test_fileno
    self.assertTrue(f.read(), "reading from file created using fd "
AssertionError: None is not true : reading from file created using fd returned 
by fileno failed

A less serious and long-standing problem with the test is that it attempts to 
close the socket twice. We are just lucky that socket.close() is called second, 
which does not raise any errors: 
<https://hg.python.org/cpython/annotate/8938b3132caa/Modules/socketmodule.c#l259>.

Regarding the purpose and use cases of fileno(), I agree with Senthil that 
using it to read the HTTP response behind the HTTPResponse object’s back in 
Python 3 is a bad idea, and I don’t think it is practical to make this work 
without losing the benefits of buffering. But there are probably other valid 
use cases such as calling getsockname() on the socket, or sending and receiving 
non-HTTP data after setting up a CONNECT tunnel.

Proposals:

1. Change the test to do use socket(fileno=...), rather than os.fdopen(...), so 
that it will be usable on Windows.

2. Ensure that the secondary socket object is not closed; use socket.detach()

3. Rewrite the test to test http.client directly, rather than indirectly 
through urlopen(). As far as I can see the purpose is only to test 
HTTPResponse.fileno(), not urlopen().

4. Rewrite the test to test a local server run in a background thread, rather 
than relying external web sites (currently Google, previously IETF, and 
Python). This would eliminate the need for setting a timeout.

5. Rewrite to the test for a more realistic use case that does not depend on 
specific internal HTTPResponse buffering and the HTTP protocol. I suggest 
mocking a CONNECT request, and uploading some non-HTTP data through the proxy.

----------
components: +Tests
keywords: +buildbot -patch
nosy: +haypo, martin.panter
stage: test needed -> needs patch
title: test_fileno of test_urllibnet intermittently fails when using 
www.example.com -> test_fileno of test_urllibnet intermittently fails
type:  -> behavior
versions: +Python 3.6 -Python 3.4

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue21069>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to