Skip to content
This repository has been archived by the owner on Dec 11, 2021. It is now read-only.

Migrate reminder endpoints over from site, set up the testing suite #30

Closed
wants to merge 10 commits into from
Closed

Migrate reminder endpoints over from site, set up the testing suite #30

wants to merge 10 commits into from

Conversation

D0rs4n
Copy link
Member

@D0rs4n D0rs4n commented Nov 20, 2021 •
edited
Loading

Closes #22
Closes #31

  • This PR adds substantial pytest fixtures that can be used in the future. (More about that topic)
  • It also introduces changes in the linting-testing workflow:
    • Add a test database to the workflow
  • Migrate reminders endpoint
    • Now, using Async SQLAlchemy and 2.0 syntax.
  • Furthermore, Database enhancements have been implemented.
  • Introduce a new task to run tests inside docker. (See the updated README)

@D0rs4n
Migrate Reminder endpoints from the site API
d764e94
@D0rs4n
Implement Asynchronous Reminder tests.
6f6c60a 
- Add an Init sql script to create a test database that can be used for testing purposes.
- Create a pytest 'conftest' with global fixtures for future testing purposes
- Implement Reminder tests in an async manner.
 - Add an additional fixture to properly access instance attributes in async pytest fixtures
@D0rs4n
Provided default values in various database models
af7c746
@D0rs4n
Add the test database to the testing workflow
272bd48
@D0rs4n
Increase code consistency in the test suite
edf9fb0
@D0rs4n D0rs4n added area: back-end area: CI Continuous Integration and Continuous Deployment area: endpoint Related to an endpoint/route area: unit tests type: feature A request for or implementation of a new feature labels Nov 20, 2021
@D0rs4n D0rs4n mentioned this pull request Nov 20, 2021
Open
onerandomusername
onerandomusername suggested changes Nov 22, 2021
View reviewed changes
Copy link

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay for what it does.

Would like to see a few sentences in the readme to clarify that the database must exist to run the tests at this point.

OR as discussed on discord, recommend running the tests on docker if support is added. 👍

.github/workflows/lint-test.yml Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@D0rs4n
Add support to run tests in docker
8404227 
- This commit adds a task that runs the tests inside docker
- It also introduces changes to the README regarding the instructions of how to run tests.
@D0rs4n
Add active field to the Reminder patch endpoint
2f62bf5 
- This commit also patches a minor issue regarding how pydantic and FastAPI parses the `expiration` field when issuing a PATCH request.
@jchristgit jchristgit self-assigned this Nov 26, 2021
@jchristgit jchristgit self-requested a review November 26, 2021 19:22
Shom770
Shom770 approved these changes Nov 29, 2021
View reviewed changes
Copy link

@Shom770 Shom770 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Maybe a few more places that could use more comments/explanation but otherwise it looks good to me

@D0rs4n
Copy link
Member Author

D0rs4n commented Nov 29, 2021

LGTM! Maybe a few more places that could use more comments/explanation but otherwise it looks good to me

Thank you for the review, I've addressed a few changes regarding code consistency, and comments.

@jchristgit
Copy link
Member

Closing this in coordination with Joe per us sunsetting this project.

@jchristgit jchristgit closed this Dec 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@onerandomusername onerandomusername onerandomusername requested changes

@Shom770 Shom770 Shom770 approved these changes

@jchristgit jchristgit Awaiting requested review from jchristgit

Assignees

@jchristgit jchristgit

Labels
area: back-end area: CI Continuous Integration and Continuous Deployment area: endpoint Related to an endpoint/route area: unit tests type: feature A request for or implementation of a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implementing test parallelisation Migrate reminders endpoint from the site API
4 participants
@D0rs4n @jchristgit @onerandomusername @Shom770