Skip to content
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

link_checker: Handle non-success status codes #897

Merged
merged 1 commit into from
Dec 23, 2019

Conversation

samford
Copy link
Contributor

@samford samford commented Dec 22, 2019

The can_fail_404_links() test doesn't encounter a 404 response in actuality, since the google.comys domain doesn't resolve. When the test is updated such that the response's status code is a 404, the test fails because the check_url() function doesn't handle non-success responses how the test's assertions expect. This commit updates check_url() to handle non-success responses, treating them much like errors.

If any (or all) of the non-success status code types (informational, redirection, client error, server error) should be handled in a different fashion, let me know. If check_url()'s current behavior is correct, I'll get rid of these changes and update the can_fail_404_links() test instead.

Either way, this will help with the forthcoming test changes related to HTTP mocking. I already have those changes finished (#898) but it will fail the can_fail_404_links() test until this is addressed.

The can_fail_404_links() test doesn't encounter a 404 response in
actuality, since the google.comys domain doesn't resolve. When the
test is updated such that the response's status code is a 404, the
test fails because the check_url() function doesn't handle
non-success responses how the test's assertions expect. This commit
updates check_url() to handle non-success responses, treating them
much like errors.
"Informational status code ({}) received",
response.status()
))
} else if response.status().is_redirection() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we not following redirections?

@Keats Keats merged commit 80327c0 into getzola:next Dec 23, 2019
@Keats
Copy link
Collaborator

Keats commented Dec 23, 2019

I'll merge that but let's check about following redirections.

@samford samford deleted the link-checker-non-success-codes branch December 23, 2019 13:25
@samford
Copy link
Contributor Author

samford commented Dec 23, 2019

I created a test site with a link to https://google.com (which will redirect to https://www.google.com) and zola check followed the redirection.

Checking site...
Checking 0 internal link(s) with an anchor.
Checking 1 external link(s).
> Checked 1 external link(s): 0 error(s) found.
-> Site content: 0 pages (0 orphan), 0 sections
Done in 4.6s.

As a result, it seems like the else if response.status().is_redirection() branch may never be encountered. I'll throw together a quick test server when I get a chance, to see how the link checker behaves under a variety of circumstances. After that, I'll update the check_url() function and create a new PR but this should be fine for the time being.

Keats pushed a commit that referenced this pull request Feb 3, 2020
The can_fail_404_links() test doesn't encounter a 404 response in
actuality, since the google.comys domain doesn't resolve. When the
test is updated such that the response's status code is a 404, the
test fails because the check_url() function doesn't handle
non-success responses how the test's assertions expect. This commit
updates check_url() to handle non-success responses, treating them
much like errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants