-
Notifications
You must be signed in to change notification settings - Fork 264
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
Added unit tests for better code coverage (#368) #431
Conversation
Please resolve conflicts 😄 |
c502cb7
to
5e563d9
Compare
.travis.yml
Outdated
@@ -10,8 +10,8 @@ install: | |||
before_script: | |||
- flake8 . --count --max-complexity=15 --show-source --statistics | |||
script: | |||
- python app/server.py > /dev/null & | |||
- pytest --capture=sys --showlocals |
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.
Why remove —showlocals? I find that option quite helpful.
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 will add the showlocals
option. I guess, I overlooked this while fixing conflicts, apologies for that
Added dependency on mock in requirement.txt Updated server.py to use absolute imports Changed the invocation of server.py to 'python -m app.server' in .travis.yml
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.
Nice work!!
* Fixes fossasia#431 * Fixes fossasia#431
Codecov Report
@@ Coverage Diff @@
## master #431 +/- ##
==========================================
+ Coverage 47.26% 91.06% +43.8%
==========================================
Files 17 28 +11
Lines 402 627 +225
==========================================
+ Hits 190 571 +381
+ Misses 212 56 -156
Continue to review full report at Codecov.
|
Added dependency on mock in requirement.txt Updated server.py to use absolute imports Changed the invocation of server.py to 'python -m app.server' in .travis.yml
mojeek.py. parsijoo.py, quora.py, yahoo.py, youtube.py (fossasia#368)
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.
LGTM ! Good work @dilraj45 .
@bhaveshAn @vaibhavsingh97 Please review |
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.
Instead of raising AssertionErrors, please adopt the bandit.yml change from #385 and then go back to using assert statements. The tests will be easier to understand that way.
Added dependency on mock in requirement.txt Updated server.py to use absolute imports Changed the invocation of server.py to 'python -m app.server' in .travis.yml
mojeek.py. parsijoo.py, quora.py, yahoo.py, youtube.py (fossasia#368)
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.
Please squash your commits.
Checklist
master
branch.Changes proposed in this pull request:
server.py
Screenshot of current coverage report