-
Notifications
You must be signed in to change notification settings - Fork 2
Huston, we have unit tests #25
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
Conversation
|
In the GitHub actions, we do run the python tests twice. Once on PR and once on merge to main. While this is repetitive, the action failing on a PR will still allow the PR to go through while the action falling on merge prevents the code from being deployed to production. Essentially the first action exists to show the reviewer/coder if the code passes all current tests while the second exists to protect our production. |
ActuallyTaylor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I would recommend using an Enumeration for your Error codes, but besides that I think the code looks good.
|
|
||
| db_connection = create_db_connection()["configs"] | ||
| if not authenticate_user(auth_token, self.db): | ||
| return {"authError":"unauthenticated user"}, 401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend an Error Enum that can contain all of your errors. I have found it is a whole lot easier for newer members, and promotes better readability and a higher level of consistency in error reports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good idea. I implemented and HTTP enum class that has default error messages.
| sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) | ||
| from server import create_app | ||
|
|
||
| ## Welcome to the conftest file for our integrated python testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the comments :)
|
|
||
| def test_bike_config_read_outdated(client): | ||
| response = client.get("/ConfigData/1") | ||
| assert response.status_code == 401, "Outdated user allowed access" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error code enum would also help here with ensuring the right errors are coming out of the server!
This code implements a basic outline of backend unit tests and a CI/CD pipeline to to push our code to GitLab where it will ultimately get deployed to our server.