Alexander Pace (9a713ac2) at 10 Mar 21:35
catch decoding error (barf) and return message to user
Removing some from __future__
imports which haven't been relevant since Python 2 support was dropped.
Daniel Wysocki (364cd7c4) at 04 Dec 22:48
Removing future imports left over from Python 2
Removing some from __future__
imports which haven't been relevant since Python 2 support was dropped.
Removing some from __future__
imports which haven't been relevant since Python 2 support was dropped.
I ended up just skipping the one test that would have needed a custom x509 key/cert. Too much hassle for basically no benefit.
We're now back to a lot of errors about failed responses from GraceDB, despite mocking up request
, get
, post
, etc. For the existing non-doctest unit tests, simply mocking up request
was sufficient, so something weird is going on.
After much pain due to a leftover from io import open
from Python 2 messing up my mocks, I now have most files being mocked up properly. I still need to mock up certificates and keys, which I should be able to leverage the existing pytest fixtures x509_key
and x509_cert
to accomplish.
Here's a StackOverflow post on this very subject. I'll wait till tomorrow to continue this.
On closer inspection, the number of failed tests actually increased, and most of them seem to be GraceDb()
failing again, now because it can't open a certificate file.
Turns out they already thought of this and unittest.mock.mock_open
already exists. This seems to have fixed all of the open('/path/to/made/up/file.png')
calls. Though we should be careful to make sure this isn't causing other issues (like if we open a real file at some point and the contents just read "MOCK DATA"
).
The tests are now failing for expected reasons like '/path/to/plot.png'
not existing. With some targeted use of Mock
objects I bet most of these tests will pass, and then it's a matter of fixing the actual issues with docstrings.
@daniel.wysocki, thanks for following up, AFAICT yes this is is resolved and the client now works on windows.
The client package now crashes immediately on Windows, I think since b68da6ee:
In [1]: from ligo.gracedb.rest import GraceDb
---------------------------------------------------------------------------
ImportError Traceback (most recent call last)
<ipython-input-1-c1e8e65ab6ae> in <module>
----> 1 from ligo.gracedb.rest import GraceDb
~\Miniconda3\envs\py39\lib\site-packages\ligo\gracedb\rest.py in <module>
26 from .exceptions import HTTPError
27 from .utils import event_or_superevent, dict_to_form_encoded, get_mimetype
---> 28 from .client import GraceDBClient
29
30 DEFAULT_SERVICE_URL = "https://gracedb.ligo.org/api/"
~\Miniconda3\envs\py39\lib\site-packages\ligo\gracedb\client.py in <module>
24 from requests import Session
25 from .extern.safe_netrc import netrc
---> 26 from os import getuid
27 from .version import __version__
28 from .adapter import GraceDbCertAdapter
ImportError: cannot import name 'getuid' from 'os' (C:\Users\spxdmm\Miniconda3\envs\py39\lib\os.py)
I have an implementation of the X.509 finder that works on windows, would you (@alexander.pace) be willing to just use that function if I can package it up and release it properly?
@duncanmmacleod did #31 (closed) fix the issue like you'd hoped? Can we close this or are there still problems on Windows?
I think I've made some good progress on this in !87. Still a bit more to go, but I should have something finished today before I disappear for the holiday.
doctest
GraceDb
(to avoid making actual requests, except for fetching some initial data, which it restricts to gracedb-test
) and builtins.open
(to pretend files like /path/to/fake/file.xml
were real)from io import open
in rest.py
open
was different from io.open
. Now they are aliases.Some important caveats:
doctest:+SKIP
directive through a trailing in-line comment. This is a potential area for future improvement, but probably fine as-is. Things currently being skipped:
GraceDb.files()
method would have required a bit more mocking, so I skipped insteaddoctest
Sphinx extension
.. doctest::
directive before each exampleGraceDb
class and builtins.open
(probably possible through Sphinx's conf.py
doctest
example is potentially creating a new GraceDb
session, if it calls GraceDb
. We're already doing this in the other unit tests, so it's probably not a huge amount of extra overhead, but there are quite a few of these so we should be careful that we're not negatively impacting service performance.GraceDb.camelCase
versions of methods, which are backwards compatible aliases to our newer, preferred GraceDb.snake_case
versions. We probably want to change all of these to avoid encouraging use of old names. We could even cause the unit test to fail if an alias is used instead of the original name.An issue I immediately ran into investigating the failures is that many examples are modifying non-test superevents on production GraceDB (though I haven't spotted any real events yet) and obviously we don't want that. Our existing unit tests mock up the client with
@pytest.fixture
def safe_client():
"""A client class which has its request() method mocked away"""
client = GraceDb()
client.request = mock.Mock()
return client
Trying to find a similar solution for doctest
, I can't find anything that integrates with pytest
, but I did get our new AI overlord to barf up a starting point for a custom doctest
runner that will need to be run separately from the pytest
command line.
# incomplete example 1: only works on a single module
import doctest
from unittest.mock import Mock
class CustomDocTestRunner(doctest.DocTestRunner):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.mocks = {} # Store your mocks here
def run(self, test, compileflags=None, out=None, clear_globs=True):
# Set up your mocks here
self.mocks['sensitive_function'] = Mock()
# Add the mocks to the test globs
if test.globs:
test.globs.update(self.mocks)
# Run the test
return super().run(test, compileflags, out, clear_globs)
# Use the custom runner in your tests
doctest.testmod(runner=CustomDocTestRunner)
# incomplete example 2: works on lots of modules but doesn't write the actual mocks
# my_doctests.py
import doctest
import mymodule # replace with your module name
class MyDocTestRunner(doctest.DocTestRunner):
def run(self, test, compileflags=None, out=None, clear_globs=True):
# Set up your mocks here
# ...
return super().run(test, compileflags, out, clear_globs)
if __name__ == "__main__":
# Find tests in mymodule
tests = doctest.DocTestFinder().find(mymodule)
# Use MyDocTestRunner to run the tests
runner = MyDocTestRunner()
for test in tests:
runner.run(test)
I'm going to keep messing around with this to see if I can get something working.
You were right, I got it to ignore conf.py
and it ran (albeit with a ton of errors -- not unexpectedly since the tests weren't written to work with doctest).
(venv) gracedb-client$ PYTHONPATH=${PYTHONPATH}/ligo python -m pytest --cov ligo.gracedb --junitxml=junit.xml --doctest-modules --ignore-glob="docs/source/conf.py"
...
======================================== short test summary info =========================================
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.addEventToSuperevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.addTag
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.add_event_to_superevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.add_pipeline_preferred_event
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.add_tag
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.confirm_superevent_as_gw
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.createEvent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.createSuperevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.createVOEvent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.create_event
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.create_signoff
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.create_superevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.create_voevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.delete_signoff
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.emobservations
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.event
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.events
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.files
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.labels
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.logs
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.modify_permissions
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.numEvents
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.num_events
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.permissions
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.removeEventFromSuperevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.removeLabel
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.removeTag
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.remove_event_from_superevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.remove_label
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.remove_pipeline_preferred_event
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.remove_tag
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.replaceEvent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.replace_event
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.signoffs
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.superevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.tags
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.updateSuperevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.update_grbevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.update_signoff
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.update_superevent
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.voevents
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.writeEMObservation
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.writeLabel
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.writeLog
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.write_emobservation
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.write_label
FAILED ligo/gracedb/rest.py::ligo.gracedb.rest.GraceDb.write_log
================= 47 failed, 481 passed, 8 skipped, 108 deselected, 3 warnings in 59.63s =================
Now to work through the failed doctests one-by-one.