Skip to content
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

Merged
merged 61 commits into from
May 1, 2023
Merged

Conversation

windoverwater
Copy link
Collaborator

@windoverwater windoverwater commented Apr 25, 2023

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.

windoverwater and others added 27 commits April 5, 2023 15:54
Co-authored-by: Ion Y <[email protected]>
Co-authored-by: Ion Y <[email protected]>
windoverwater and others added 19 commits April 30, 2023 15:29
@windoverwater
Copy link
Collaborator Author

@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.

@ion-oset
Copy link
Contributor

ion-oset commented May 1, 2023

@ion-oset - thanks for the awesome review - appreciate it +1 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

👍🏼. I can explain that when we discuss it.

  1. ditto for test_main.py - I don't yet grok fixtures and tests in the design or implementation space - anything you say is good

👍🏼 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.

  1. 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?

Yes.

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)?

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.

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.

We need both realistically. This should be a primary agenda item when we speak next (or we can discuss in Discussions).

@ion-oset
Copy link
Contributor

ion-oset commented May 1, 2023

Assuming all the non-test changes don't break anything LGTM.
If the tests break I commit to fixing them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add code to integrate the first two endpoints with the backend
2 participants