QCElemental icon indicating copy to clipboard operation
QCElemental copied to clipboard

pytest requires certifi

Open mbanck opened this issue 4 years ago • 5 comments

Describe the bug

Running pytest-3 I get spurious SSL certifications errors like:

/usr/lib/python3.9/ssl.py:1040: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <ssl.SSLSocket [closed] fd=-1, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6>
block = False

    @_sslcopydoc
    def do_handshake(self, block=False):
        self._check_connected()
        timeout = self.gettimeout()
        try:
            if timeout == 0.0 and block:
                self.settimeout(None)
>           self._sslobj.do_handshake()
E           ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1123)

/usr/lib/python3.9/ssl.py:1309: SSLCertVerificationError

Expected behavior

Tests should pass or be skipped. If I install the certifi python package, the tests do succeed. I think setup.py should have certifi as a requirement and/or the tests should otherwise abort with a clear(er) error if certifi isn't there / SSL certificates aren't installed so https requests fail.

mbanck avatar Jan 10 '21 11:01 mbanck

What's pytest-3 or what test(s) are you failing on? qcel requires pytest >=4. If I remove the certifi module, I'm still getting a clean test suite.

I do see that some of the pubchem tests are missing the @using_web decorator, so I'll fix that.

loriab avatar Jan 28 '21 23:01 loriab

I've removed the lingering reference to pytest 3 in setup.py, in case that was causing the confusion. Also, all the pubchem tests are properly escaped, so they should be avoidable. I can add certifi if you still see the problem, but please give this a try first.

loriab avatar Jan 30 '21 03:01 loriab

What's pytest-3 or what test(s) are you failing on?

It's pytest; I gues Debian renamed the executable to pytest-3 to (back in the days) distinguish it from the python-2.x version, and stuck with it. I didn't realize it is a Debianizm, sorry (nor do I know a lot about pytest).

qcel requires pytest >=4. If I remove the certifi module, I'm still getting a clean test suite.

pytest is at 4.6.11.

If I remove the certifi module now, I don't get the SSL certificate errors anymore either, so maybe it was a transient error, but see below. Do you want to close this and should I open a new issue?

I do see that some of the pubchem tests are missing the @using_web decorator, so I'll fix that.

Thanks, I mentioned that in https://github.com/MolSSI/QCElemental/issues/83#issuecomment-757475240 but never got around to file a new issue for it.

In any case, I still get failures (also when cherry-picking cddeb41a) like this, so the @using_web does not seem to work:

qcelemental/molparse/from_string.py:307: 
[...]
E           qcelemental.exceptions.ValidationError: 	PubchemError
E           <urlopen error [Errno 111] Connection refused>
E           	received when trying to open
E           	https://pubchem.ncbi.nlm.nih.gov/rest/pug/compound/cid/223/property/IUPACName,MolecularFormula,Charge/JSON
E           	Check your internet connection, and the above URL, and try again.

See https://buildd.debian.org/status/fetch.php?pkg=qcelemental&arch=all&ver=0.17.0%2Bdfsg-2&stamp=1610282048&raw=0 for the full build log.

mbanck avatar Jan 30 '21 18:01 mbanck

huh. perhaps the error catching is misaligned with exactly the connectivity lack of the Debian build server settings. Would pytest -k "not pubchem" be ok for the moment?

also, I'll be minting a v0.18.0 next week if you want to work from master.

loriab avatar Jan 30 '21 19:01 loriab

I took a hammer and just made the internet_connection return False in a Debian patch for now, but I'll try the not pubchem, thanks.

In the end it would be interesting to figure out what the connection refused is coming from (there's some weird host = '127.0.0.1:9' message in the logs and also when I try locally), but it works for now.

mbanck avatar Jan 30 '21 20:01 mbanck