-
Notifications
You must be signed in to change notification settings - Fork 5
added test for duckduckgo #28
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
base: master
Are you sure you want to change the base?
Conversation
I proactively looked into the search engines another contributor has added but not yet been merged and created a test for one of those search engines
adding test for pySearch for duckduckgo
jrkong
left a comment
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 like the work on the scraper so far but let's keep this work out of this PR so this PR is only about the test you added.
| import urllib | ||
| import webbrowser | ||
| from urllib.request import urlopen | ||
| from bs4 import BeautifulSoup |
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 trying to achieve with BeautifulSoup? It's probably useful but can you put this under a different pull request? Let's keep this one to just the test for now.
| html = self.url; | ||
| req = urllib.request.Request(html, headers={'User-Agent': 'Mozilla/5.0'}) | ||
|
|
||
| soup = BeautifulSoup(urlopen(req).read(),"html.parser") |
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 have a feeling this is part of another PR that you're working on. I think this conversation should occur on another issue however I do like the direction you're taking it in. Going off of what I can read I think you're trying to scrape the links from the site to populate some results. This is great but I don't think it should be part of the base functionality.
I think this should be put behind a command line argument for now so the users can choose between simply opening the link or having a few results pop up.
Regardless, this is a bit off topic for this PR. Let's just keep this particular PR about the test so the changes are easier to track.
|
Hello @shreenaathia! Thanks for updating the PR.
|
No description provided.