-
Notifications
You must be signed in to change notification settings - Fork 0
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
Phone book app #2
base: master
Are you sure you want to change the base?
Conversation
what do you think about writing the summary line and description of what you have done in the imperative mode, that is as if you were commanding someone. Start the line with "Fix", "Add", "Change". |
#test add contact method | ||
def test_add_contact(self): | ||
rv = self.app.post('/add_contact', data=dict(name='<anything>', phone='<anything>'), follow_redirects=True) | ||
assert 'anything' |
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 like your naming convention, keep it up. On the other hand, what do you think about including the expected results? The expected result tells the tester what they should experience as a result of the test steps. This is how the tester determines if the test case is a “pass” or “fail”.
phone = request.form['phone'] | ||
store['name'] = name | ||
store['phone'] = phone | ||
phone_book.insert(0,store) |
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 like the flow of execution. What do you think about making your code DRY and applying MVC?This will enable other developers to easily understand your code and do maintenance. DRY will also allow some codes to be reusable.
Resolve the conflicts here |
Added TDD to my app