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

Db tests #89

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Db tests #89

wants to merge 3 commits into from

Conversation

rajiqxf2
Copy link
Collaborator

@rajiqxf2 rajiqxf2 commented Aug 12, 2021

Added following unit tests to validate newsletter db

  1. tests to validate newsletter db schemas
  2. tests to validate pre-defined article categories
  3. tests to validate all table names

Context of the work

  • Firstly , I tried using sqlalchemy's metadata to verify table schemas . It did not fit because unit test assertion fails to validate schema objects .Unittests assertion validates object references instead of values and Fails .

  • Next solution is to try sqlalchemy's inspect which worked with some work around and here also list assertion Fails at type comparision [{'type':INTEGER}] , so I converted list to string for assertion to work as it is expected

Note: test_db has been binded and created to store expected db models to compare with actual newsletter db models , I chose to create test_db instead of storing expected db objects into variables as I thought its easy to read and maintain test_db for any updates required in the future

@sravantit25
Copy link
Collaborator

@mohanqxf2 I believe the required information is present to do the review. Please complete your review and assign the PR back to me.

@mohanqxf2
Copy link
Collaborator

pardon me @rajiqxf2 @sravantit25 I missed the notification. here are my observations.
the tests look good, Initially, i was having issues with running the tests, later figured out qa_db database need to be created, before running the db_tests.py
I created a setup on my local and ran the tests, all tests passed.
dbTests_NewsletterAutomtion

  • Please update the readme file.

@sravantit25
Copy link
Collaborator

@rajiqxf2 I am really very sorry for holding this for so long. Please go ahead to merge it.

@qxf2
Copy link
Owner

qxf2 commented Mar 5, 2023

@rajiqxf2 - can you please resolve conflicts here and send to me? It looks easy enough because the conflicts are in one file only.

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

Successfully merging this pull request may close these issues.

4 participants