-
Notifications
You must be signed in to change notification settings - Fork 47.8k
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
Add automated testing #93
Conversation
The goal of this change is to have Travis-CI automatically run [flake8](http://flake8.readthedocs.io) tests on every pull request. This will help contributors know if their submissions are going to *break the build*. To turn on this **free** service, you would need to do steps 1 and 2 of https://docs.travis-ci.com/user/getting-started/
Is Trusty needed for this? If not, I would omit the first two lines. |
You could also omit the patch versions. |
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.
People are going to assume that files with filenames that end in .py contain valid, executable Python code. Flakes8 verifies this. If the filename ends in .py then flake8 is going to scan the file looking for and flagging any text which is not valid Python code. For your files to get a passing grade you have at least three options:
|
#98 demonstrates how we can modify the code to be lintable but still clearly indicate where the user needs to add code. |
This is what the output looks like before #98 |
Discovered via donnemartin#93
I understand that we are treating these files largely as pseudocode but I still believe that this PR is required. It is a distraction for readers who know Python if the psudocode that they are reading contains syntax errors and variable names that do not match up. |
The output is looking better but we still have to correct one syntax error and 10 undefined names. #149 Resolves the syntax error and 4 of the undefined names. |
#149 has been merged so the syntax error is resolved and we are down to seven undefined names.
flake8 testing of https://github.com/donnemartin/system-design-primer on Python 3.6.3 $ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics
|
__levels__ is an _undefined name in this context (see donnemartin#93) so replace it with __self.levels__ which is defined a few lines above.
In the current code, __seller__ is an unused variable and __category__ is an undefined name (see donnemartin#93). Given that the class is __SpendingByCategory__, this PR advocates changing seller —> category.
After more merges, we are down to just
flake8 testing of https://github.com/donnemartin/system-design-primer on Python 3.6.3 $ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics
|
Allows donnemartin#93 to be merged and the Travis CI tests to pass even before a solution to donnemartin#148 is proposed.
donnemartin#162 Temporarily suppress flake8 testing of social_graph_snippets.py
* Fixes donnemartin#148 * Enables donnemartin#93 * Replaces donnemartin#162
Given recent pull requests, these flake8 tests pass. |
@polyzen Can I please get your review on these proposed changes?
|
Bump on this one? |
The goal of this change is to have Travis-CI automatically run flake8 tests on every pull request. This will help contributors know if their submissions are going to break the build. To turn on this free service, you would need to do steps 1 and 2 of https://docs.travis-ci.com/user/getting-started/