-
Notifications
You must be signed in to change notification settings - Fork 176
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 test for urlhaus #604
added test for urlhaus #604
Conversation
Urlhaus Unit Tests should be added to .github/workflows/CI-production-testing.yml and .github/workflows/CI-staging.yml |
I have already added it to production_testing.yml just forgot about CI-staging.yml Sorry i will fix it |
Signed-off-by: Sebastian Garcia <eldraco@gmail.com>
Signed-off-by: Sebastian Garcia <eldraco@gmail.com>
Signed-off-by: Sebastian Garcia <eldraco@gmail.com>
…r-macos Fix-docker-macos
…parsing_whitelist whitelist: fix parsing commented lines
tests/test_urlhaus.py
Outdated
mock_response = Mock(status_code=status_code) | ||
mock_response_instance.post.return_value = mock_response | ||
|
||
urlhaus = URLhaus(mock_db) |
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 should be moved to module_factory, always remember no object ever used in the tests should be created in a test_*.py file, because you'll need to recreate it again later and that would cause code duplication
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.
check any object creation in all functions for the same issue ok?
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.
Sorry i thought you were only asking about the object which were being used as fixture in the code
i am fixing it.
tests/test_urlhaus.py
Outdated
) as mock_create_session: | ||
mock_create_session.side_effect = requests.exceptions.ConnectionError | ||
with pytest.raises(requests.exceptions.ConnectionError): | ||
urlhaus.make_urlhaus_request(to_lookup) |
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.
what are you testing here? shouldn't we be checking that the return in None in case of a Connection error? also why did you add a side effect for create_urlhaus_session() ? isnt this mock_response_instance.post.side_effect
enough?
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.
@AlyaGomaa I am so sorry for this is incomplete i was trying something else here.It did not go as planned then i forgot to change it
hey @AlyaGomaa sorry for the multiple commits please only review the last three PRs |
Fixes Issue #603
Changes proposed
Added more tests for urlhaus
Check List (Check all the applicable boxes)