-
Notifications
You must be signed in to change notification settings - Fork 53
Fix for #155 #157
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
Fix for #155 #157
Conversation
Thanks for the contribution, @terrameijar ! It looks like the build is failing in flake8 because flake8 runs in python2 instead of 3. I'm opening a PR to clean up the This build should pass after those changes are merged. |
axe_selenium_python/axe.py
Outdated
""" | ||
Write JSON to file with the specified name. | ||
:param name: Name of file to be written to. |
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 doc string should be updated here.
string += "\n\n\n" | ||
return string | ||
|
||
def write_results(self, data, name="results.json"): |
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.
There should be a default parameter so that write_results
can be called without a filename or path.
axe_selenium_python/axe.py
Outdated
:param output: JSON object. | ||
""" | ||
filepath = os.path.join(os.getcwd(), name) | ||
filepath = os.path.abspath(name) |
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.
If we set the default value of name to None, we could add a check to write to the current directory when name is undefined.
something like:
def write_results(self, data, name=None):
filepath = os.path.abspath(name) if name else os.path.join(os.getcwd(), name)
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.
Done, I wrote the same over multiple lines because I got a "line to long" error in the linter when I wrote everything on one line.
axe_selenium_python/axe.py
Outdated
except NameError: | ||
f.write(json.dumps(data, indent=4)) | ||
except FileNotFoundError as fnf_error: | ||
print(fnf_error) |
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.
A better experience would be to create the file if it doesn't exist.
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.
This section can be removed. @kimberlythegeek suggested that we write to the current directory if there's no path passed to write_results
. I intend to do something like open(filepath, 'w')
This will make sure that a file is always created, either in the current directory or in the user specified path. I'll submit a new commit now with changes.
@kimberlythegeek I added another commit to address the changes you requested. Please review when you have a moment. |
thank you @terrameijar ! |
This is a proposed fix for #155
I changed the
name
keyword argument to a positional argument to enable the user to pass in whatever path they like.