[WIP] adding crawler functions to testing #103
Conversation
| @@ -0,0 +1,41 @@ | |||
| import os | |||
There was a problem hiding this comment.
@marco-c Can we have a separate library for driver which can be used in test cases as well as in actual crawler ?
There was a problem hiding this comment.
Definitely, you can put it in autowebcompat.
Yes, the approach looks good to me overall! |
marco-c
changed the title
poush
force-pushed
the
fix75
branch
3 times, most recently
from
089fa47 to
c95387a
Compare
Codecov Report
@@ Coverage Diff @@
## master #103 +/- ##
=======================================
Coverage 18.95% 18.95%
=======================================
Files 16 16
Lines 1931 1931
Branches 328 328
=======================================
Hits 366 366
Misses 1562 1562
Partials 3 3Continue to review full report at Codecov.
|
autowebcompat/crawler.py
Outdated
| @@ -0,0 +1,18 @@ | |||
| from selenium.common.exceptions import NoAlertPresentException, NoSuchWindowException | |||
There was a problem hiding this comment.
@marco-c What do you suggest for the file this file's name? or it is good to move this function to utilis.py
|
@marco-c Also, can I break up the task of adding tests cases? So taking the changes done in PR as one part and rest is completed by creating issues or it should be better if it completes in one PR? |
Sure, please file an issue for anything that should be tested that is not covered in this PR. |
|
@poush do you still intend to work on this? |
|
Closing for lack of response. Feel free to reopen if you still intend to work on this in the future. |
|
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. |
|
@marco-c can you please reopen it? I would like to continue on it. |
|
Sure! |
different fixes different fixes
|
ping @poush |
|
😅Got busy in my summer project. Give me a week or two. I will fix the travis issues. |
poush
force-pushed
the
fix75
branch
2 times, most recently
from
d46ca3c to
5f7af1f
Compare
|
@marco-c test seems passing, please have a look. I will put into the issue what next methods will be covered in next PR. |
poush
force-pushed
the
fix75
branch
2 times, most recently
from
f05ab76 to
35f5424
Compare
|
@marco-c Can you please check it again? I fixed all the reviews that same day. |
There was a problem hiding this comment.
Oh sorry, I hadn't realized you fixed the nits! Please tell me whenever you make changes to the PR, as it looks like GitHub doesn't notify me when you add more commits.
BTW, the patch looks good again, but there are a few arguments that are missing for the driver.
| chrome_options.add_argument('--headless') | ||
| chrome_options.add_argument('--window-size=412,732') | ||
| chrome_options.add_argument( | ||
| '--user-agent=Mozilla/5.0 (Linux; Android 6.0.1; Nexus 5 Build/M4B30Z) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.83 Mobile Safari/537.36') |
There was a problem hiding this comment.
It looks like some arguments are missing here.
| chrome_bin = 'tools\\Google\\Chrome\\Application\\chrome.exe' | ||
| nightly_bin = 'tools\\Nightly\\firefox.exe' | ||
| else: | ||
| chrome_bin = nightly_bin = None |
There was a problem hiding this comment.
Let's throw an exception here, as this should never happen.
| def setup_method(self): | ||
| self.driver = Driver() | ||
|
|
||
| def teardown_method(self): |
There was a problem hiding this comment.
Nit: I think you can remove this function if it's not doing anything.
| chrome_bin, nightly_bin = utils.get_browser_bin() | ||
| assert isinstance(chrome_bin, str) | ||
| assert isinstance(nightly_bin, str) | ||
| assert os.path.exists(os.path.join(os.path.abspath('./'), chrome_bin)) |
There was a problem hiding this comment.
Nit: I wonder if you can't just do assert os.path.exists(chrome_bin).
3 participants
Work in progress for #75
@marco-c Sorry for delay! I was busy in exams. So, till now what I have done is creating a class for setting up the selenium drivers and provided a sample use of it though one test case. I would like to know if this is good method or there are some changes can be made to make it better. If it looks good then other possible functions can be added easily.