-
Notifications
You must be signed in to change notification settings - Fork 4
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 tests to cover running simple query #8
base: master
Are you sure you want to change the base?
add tests to cover running simple query #8
Conversation
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 LogSearch.search accepts 8 params. How about adding test cases for valid/invalid values of the params?
test/basic_test.py
Outdated
text='ok' | ||
) | ||
ls = LogSearch(api_key='dummy_key') | ||
m.get("https://eu.rest.logs.insight.rapid7.com/management/organizations/apikeys", |
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.
How about extracting global string const for base URL?
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 can see that you did that, so I'm gonna copy yours
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
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.
Actually there are some issues with build for python 3.x.
yeah...I see |
b637331
to
312d776
Compare
pytest.ini
+pytest-pythonpath
added@seamusc