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

tests(*) increase robustness of httpbin-reliant tests #629

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Nov 26, 2024

We sometimes get a real 503 from httpbin.org as well. The tests still succeed in case of upstream errors.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.86064%. Comparing base (9136e46) to head (e175b8c).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##                main        #629   +/-   ##
=============================================
  Coverage   90.86064%   90.86064%           
=============================================
  Files             52          52           
  Lines          11259       11259           
=============================================
  Hits           10230       10230           
  Misses          1029        1029           
Flag Coverage Δ
unit 90.60539% <ø> (ø)
valgrind 82.53782% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@casimiro
Copy link
Contributor

Weird. The 503 would have come from the local httpbin instance that runs during CI, not the real one hosted at httpbin.org, which is unexpected.

@thibaultcha
Copy link
Member Author

Sure, but the test is still considered successful.

@thibaultcha thibaultcha force-pushed the tests/httpbin-robustness branch from bf2887d to e918992 Compare November 26, 2024 17:01
We sometimes get a real 503 from httpbin.org as well. The tests still
succeed in case of upstream errors.
@thibaultcha thibaultcha force-pushed the tests/httpbin-robustness branch from e918992 to e175b8c Compare November 26, 2024 17:02
@thibaultcha
Copy link
Member Author

In fact the error probably comes from our Nginx proxy in front. But again, we already expect this as per the test cases, this PR is not introducing anything new.

@thibaultcha thibaultcha merged commit d834bb0 into main Nov 26, 2024
32 checks passed
@thibaultcha thibaultcha deleted the tests/httpbin-robustness branch November 26, 2024 18:32
@thibaultcha
Copy link
Member Author

thibaultcha commented Nov 26, 2024

The setup of the failing run has a strange "setup local httpbin server" step in which we see:

Run docker rm -f httpbin httpbin_proxy
Error response from daemon: No such container: httpbin
Error response from daemon: No such container: httpbin_proxy

It looks like it is missing the docker run -d ... commands.

We can investigate it a bit. That said, iirc these special 5xx handling in these tests were there even before we had a local httpbin setup as httpbin.org was really giving us some 500s from time to time.

@thibaultcha
Copy link
Member Author

Actually the output is not anomalous, the docker run -d are embedded in the docker rm -f text fold... We'll need to dig the proxy logs to investigate it; but again since the tests themselves are considering passing, we may be wasting time to dig a minor issue. We still can though.

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