DM-33935: Create basic test framework for prompt-prototype #6
Conversation
| @@ -1,3 +1,5 @@ | |||
| __all__ = ["Visit"] | |||
There was a problem hiding this comment.
I didn't want to muck with the visit.py files, because we may want to consolidate those somehow (there's two copies in different places).
There was a problem hiding this comment.
This is the real one, the other is a symlink. I'm pretty sure we can delete the latter if we change the import in upload.py, I guess I was waiting for test coverage of upload before trying. 🙂
There was a problem hiding this comment.
Fair point, but I'm nervous about writing tests of upload (see KT's comment on DM-33938: upload.py is a test itself)
There was a problem hiding this comment.
I disagree -- for now, at least, upload.py is the server for which activator.py is the client; that makes it part of the system we're developing. In particular, we probably will need to make changes to upload.py as we start doing more substantial integration testing. (And some of those changes, especially anything involving the next_visit message, might well be reflected in the production system.)
| """Interface layer between the Butler middleware and the prompt processing | ||
| data handling system. | ||
| Parameters |
There was a problem hiding this comment.
Can you include an explanation of what a MiddlewareInterface object actually represents? I find it very confusing that __init__ performs Butler operations, some of them ones that change state.
The fact that some (but not all) of the code assumes multiple concurrent MiddlewareInterface objects is probably a clue...
There was a problem hiding this comment.
I've added a paragraph, but I think we don't want to assume that any of the stuff in __init__ will stick around (I suspect most of that should move to prep_butler, but don't know yet).
There was a problem hiding this comment.
Of course, and we shouldn't document implementations anyway. I just meant that the class seems to be intended to be used in particular ways, which are not at all obvious from the name.
parejkoj
force-pushed
the
tickets/DM-33935
branch
from
c285406 to
d743c39
Compare
|
Please have another look at the middleware docstrings: I've tweaked them per your suggestions. |
parejkoj
force-pushed
the
tickets/DM-33935
branch
from
d743c39 to
396cea3
Compare
Use relative imports so tests are happy.
We can use pytest-only features, so we don't need the unittest.main stuff. I tried to remove unittest entirely (no parent class), but assertLogs is much easier to use than the pytest equivalent, it appears. We might want the activator tests to be pure pytest because there are useful Flask fixtures.
parejkoj
force-pushed
the
tickets/DM-33935
branch
from
396cea3 to
309738d
Compare
2 participants
This ended up being rather trivial; I'm putting this in review while I keep working on the flask/google.cloud tests (trying to mock a non-installed package (gcloud) is tricky). I'd rather have something to run in a github action now, while we figure out what we want the tests to actually do (I can't mock the butler interface until I have a better sense of how we'll actually use it).