-
Notifications
You must be signed in to change notification settings - Fork 74
Conversation
This test doesn't use Webdriver at all and doesn't use PageObjects. |
parsed_html = BeautifulSoup(about_page.text) | ||
|
||
bad_urls = [] | ||
urls = [ancor['href'] for ancor in parsed_html.find(id='main').findAll('a')] |
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.
That's probably the weakest place - hardcoded id of main area of the page (to avoid collecting anchors from footer or header).
This looks great to me and runs as expected. I like the use of BeautifulSoup to pull the urls from the page. By not hard coding the urls this is more future proof. Thanks @sashakruglov! This will increase our coverage greatly.
|
I haven't run it but i like it. i kinda think this test should be put in the templates project and expanded to crawl thru all links with the same base_url. |
parsed_html = BeautifulSoup(about_page.text) | ||
|
||
bad_urls = [] | ||
urls = [anchor['href'] for anchor in parsed_html.find(id='main').findAll('a')] |
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.
you might want to explicitly filter out javascript urls here (the ones with href #). this project may not have them on this page, but many projects/pages do.
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.
@klrmn This rule needs additional refinements.
There is couple of links with #
on this page, but they lead to different web-site. And this sounds like a bad idea.
We can filter URLs with #
but only if they are for the same web page that is under test. So, if we are checking:
https://mozillians-dev.allizom.org/en-US/about
, we can omit URL like https://mozillians-dev.allizom.org/en-US/about#LALALA
.
I might be wrong, but this is not javascript URL. So maybe I just misunderstood your idea.
I'm going to make the call this pull request good. We can integrate @klrmn's ideas in a later pull reuqest. @sashakruglov if you would fix a few small nits I'll merge this
|
@m8ttyB fixed PEP8 violations |
LGTM |
link checking test for /about page
Addresses issue #55