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

Fix specs failing on main (ignore favicon requests, expect new error) #273

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidrunger
Copy link

@davidrunger davidrunger commented Aug 26, 2024

This change aims to fix 5 specs that are currently failing consistently on main and 2 specs that are currently flaking fairly frequently. The 5 consistent spec failures can be seen here in the output of a test run for a recently submitted PR (#272). (The failures are not related to the changes being made in that PR.)

There are two distinct issues being fixed in this PR.

Problem 1 (causes 4 consistently failing examples and 2 flaky examples): It seems that requests are now being made for /favicon.ico that the specs don't seem to expect/account for. Presumably these requests weren't being made before. There might have been some change in Chrome or elsewhere, such that these requests are maybe now made by default, whereas maybe previously they weren't being made by default.

Fix: Filter out the /favicon.ico requests (instances of Ferrum::Network::Exchange) in the tests that are currently failing due to the presence of such requests. (This approach is somewhat inelegant/cluttering, and there might be some better possible approach.)

Problem 2 (causes 1 consistently failing example): It seems that, now, if a request is made to a URL that is protected by basic authentication, but if basic authentication has not been provided for that request, then a Ferrum::StatusError exception will be raised.

Fix: Expect/rescue this error in the relevant spec that makes a request without providing credentials to an endpoint that is protected by basic auth.

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.

1 participant