-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/post pr 17 comments #20
Conversation
This reverts commit 821db9f.
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
@ion-oset - thanks for the awesome review - appreciate it 👍 My high level response is: 1) regarding the json_data_models, as I don't understand that yet and it is not yet used, anything you say is good; 2) ditto for test_main.py - I don't yet grok fixtures and tests in the design or implementation space - anything you say is good; 3) regarding main.py and backend.py will try to incorporate everything you say and get it working again. Note - for me 'working' is when all the endpoints work as expected when connected to the backend, not in mock mode. And unfortunately this is currently still manual. Can you help automate that with designing a proper test_main.py for that scenario? Regarding testing in general, I am still at a loss to describe the basic mock testing strategy. Do we run tests against VTP-web-api with it plugged into the backend (which I would like to do leading up to the demo and beyond) or without the backend plugged (in which case we would be testing the web-api, which is important but not important for the demo)? Ideally, at a minimum, I would like to run tests against the backend for the next few weeks either via the web-api or directly via the VoteTrackerPlus repo solo. |
👍🏼. I can explain that when we discuss it.
👍🏼 Same as 1) I should also emphasize: I'm not sure all my fixes to separate the tests from the fixtures will work as written, since I didn't (obviously) test them. When you merge I'll go debug them.
Yes.
I've started writing something up about how to do testing using FastAPI and pytest-mock. The short answer to your question is that we should do both, but we can focus on the demo for now. In general in PyTest which backend gets used you want boils down to putting the choice behind a fixture. Btw, not directly relevant to your question but as a clarification: We should be explicit about the difference between mock mode and mock testing, to avoid us all getting confused. It's not technically a "mock" in the testing sense if there is a "mock server" as part of the demo. Testing mocks are for white box testing which they accomplish by patching the internals of tested modules. They do not exist in a running application. Having two variants of a data source in a running system, for demo purposes is similar in intent, but it's not about testing and (importantly) it's not white box. It works via some common interface between the variants. If you knew all this sorry for repeating it.
We need both realistically. This should be a primary agenda item when we speak next (or we can discuss in Discussions). |
Assuming all the non-test changes don't break anything LGTM. |
This is a PR of the same branch as #18 but this one is to main as opposed to the previous branch. As mentioned in that one, I include all of Ion's comments in the upstream mock repo and regenerate the files in this repo.
If possible I would like the main.py and backend.py files here to be reviewed by you guys - this is the latest pivot.
Note - all 6+ endpoints work when talking to the backend. Did not get the mock'ed version to work (It may or may not) as it looks like the current idea is a bad design - waiting on a description of the desired design to handle mock testing.