[WIP] Write unit tests for the functions in utils.py #36
Conversation
| @@ -0,0 +1,15 @@ | |||
| import sys | |||
| import os | |||
| sys.path.append(os.path.dirpath(os.path.realpath(__file__)) + '/../') | |||
There was a problem hiding this comment.
Is this an acceptable way to import the utils.py module from the outer directory? Seems like a hack to me, but this is what I got after some stackoverflow searches.
There was a problem hiding this comment.
I hadn't seen this comment 😄
I've replied at #36 (comment).
There was a problem hiding this comment.
@skulltech @marco-c I think its always better to avoid system calls. :) Once the structure of the directory is set, it will be easier. OOP structure favours imports with ease.
Can I structure the whole network.py into a nice class with appropriate functions ? What do you think ?
There was a problem hiding this comment.
@skulltech @marco-c I think its always better to avoid system calls. :) Once the structure of the directory is set, it will be easier. OOP structure favours imports with ease.
For now, we are leaning towards moving the network.py and utils.py modules into a autowebcompat directory and leaving the scripts (which are never used as modules) in the top-level directory. Discussion at #36 (review).
Can I structure the whole network.py into a nice class with appropriate functions ? What do you think ?
Sounds good to me, this should happen regardless of the directory structure.
| @@ -0,0 +1,15 @@ | |||
| import sys | |||
| import os | |||
| sys.path.append(os.path.dirpath(os.path.realpath(__file__)) + '/../') | |||
There was a problem hiding this comment.
Can we avoid this? Maybe we can restructure the project like the first example at https://docs.pytest.org/en/latest/goodpractices.html#tests-outside-application-code.
There was a problem hiding this comment.
The first example's directory structure looks good. If we rearrange our project like that, will we able to import the project modules directly? like
from autowebcompat import utilsThere was a problem hiding this comment.
I think so, if you execute python3 -m pytest or PYTHONPATH=. pytest.
Instead of restructuring, we can also run PYTHONPATH=.. pytest.
There was a problem hiding this comment.
I think restructuring will also help us in the long run. So better to just do it now.
There was a problem hiding this comment.
What do you think?
There was a problem hiding this comment.
I'm not totally sure because the source files are executable scripts rather than modules that you import.
Only network.py and utils.py are actually modules.
Maybe we can restructure putting network.py and utils.py in a autowebcompat directory, but keeping the other scripts where they are.
|
Created issue #37 in reference to this. |
|
Hey @skulltech, since #6 is a "good first issue", I'll close this and let someone without experience with the project take it. |
|
Please provide your feedback on this pull request here. Privacy statement: We don't store any personal information such as your email address or name. We ask for GitHub authentication as an anonymous identifier to account for duplicate feedback entries and to see people specific preferences. |
This is a WIP as of now. Made pull request to get input from project maintainers. @marco-c