Try go-httpbin (PR #23398)
Maarten
Just a test to see if this will work. When python httpbin fails again, this could be an alternative.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/23398
Commit Summary
- a9f47d4 Try go-httpbin
File Changes
(4 files)
- M .github/workflows/ci.yml (18)
- M .github/workflows/ci_cmake.yml (10)
- M .github/workflows/ci_mac.yml (12)
- M build/tools/httpbin.sh (9)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/23398.patch
- https://github.com/wxWidgets/wxWidgets/pull/23398.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 197370e Trigger all CI builds
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- b0ef2ee dont use old go
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- fcb23b4 fix port, default was 8080
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bryan Petty
I know @vadz did attempt to use this a long time ago once. I don't recall what issues he ran into with it though.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 65ded2d Try go-httpbin
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Yes, I remembered that too, and found the below text in the mailing list. Installing works, the only remaining problem I'm looking at now is that it doesn't seem to run in AppVeyor. CURL can't connect / connection refused.
Conclusions so far:
- running go-httpbin works in all GitHub CI tests
- can't run go-httpbin in old appveyor containers, it needs at least go 1.16(?). so install it manually
- go-httpbin (and https://httpbingo.org/ ) responses are slightly different, causing the tests tofail
Also I really, really hate trying to use python/pip in a portable way, this has taken way too much time. So I tried to switch to a version not requiring Python from https://github.com/mccutchen/go-httpbin but I didn't have better luck with Go -- although here it could be due to my total lack of knowledge of it. The "go install" command given in the README didn't
work, and from the documentation it looks like "go get" should be used instead, but it still doesn't work for some reason, even after setting GO{ROOT,PATH} -- it seems to look for the files only locally instead of downloading them for some reason. If anybody knows how to install this under macOS 10.11, please let me know.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- c0da853 Try go-httpbin
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 0b306c7 something broke again
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- e6ca070 something broke again
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 85831af something broke again
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 0350747 msiexec with cmd
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- e2271ef install needs /q, see if gopath is needed in path
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 75fd98d echo some go paths
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- af3d429 GOPATH
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 3 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Closed #23398.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
success
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
If it works, perhaps we want to switch to it already, without waiting for the next problem?
I haven't compared, but what takes longer: installing Python or Go version?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Build times are all over the place, so it is hard to compare.
On GitHub CI, the setup-python and setup-go actions seem both instant. Comparing Ubuntu 22.04 wxGTK 3 then installing python httpbin takes 5s, building go-httpbin takes 20s.
Appveyor installs Go, this adds between 30s to 60s. And installing go-httpbin seems to take about 10s longer than python httpbin.
python httpbin still seems to fail randomly in one of the builds for the last commit to master: https://ci.appveyor.com/project/wxWidgets/wxwidgets/builds/46699078/job/m30bbfpcqihi6i7q#L3122
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
I'll reopen this since it still seems to fail with python: https://ci.appveyor.com/project/wxWidgets/wxwidgets/builds/46818435/job/u8vmba8bh6ey7dmo#L3120
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Reopened #23398.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks again for working on this, I'm more than willing to try using this, but have a couple of minor questions before merging it.
> @@ -228,7 +228,7 @@ TEST_CASE_METHOD(RequestFixture,
Create("/base64/VGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIHRoZSBsYXp5IGRvZw==");
Run();
- CHECK( request.GetResponse().AsString() == "The quick brown fox jumps over the lazy dog" );
+ CHECK( wxString(request.GetResponse().AsString().c_str()) == "The quick brown fox jumps over the lazy dog" );
Sorry, I am not sure why is this needed?
> @@ -263,7 +263,7 @@ TEST_CASE_METHOD(RequestFixture,
pos += strlen(expectedKey);
// There may, or not, be a space after it.
- while ( wxIsspace(response[pos]) )
+ while ( wxIsspace(response[pos]) || response[pos] == '[' )
Could you please update the comment to explain why do we need to test for [ here? I don't really understand it...
In appveyor.yml:
> @@ -86,15 +88,21 @@ before_build: build_script: c:\projects\wxwidgets\build\tools\appveyor.bat before_test: +- cmd: | + echo Getting and launching httpbin. + rmdir %GOROOT% /s /q + mkdir %GOROOT% + appveyor DownloadFile https://golang.org/dl/go%GOVERSION%.windows-amd64.msi
Just to be clear: we can't use any of Go versions already available on Appveyor?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
> @@ -186,11 +186,11 @@ TEST_CASE_METHOD(RequestFixture,
if ( !InitBaseURL() )
return;
- Create("/bytes/65536");
+ Create("/bytes/1000");
I changed this to 1000, because go-httpbin doesn't add a content-length header when the size is over 2048 bytes. Resulting in the functions below to return -1.
> @@ -228,7 +228,7 @@ TEST_CASE_METHOD(RequestFixture,
Create("/base64/VGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIHRoZSBsYXp5IGRvZw==");
Run();
- CHECK( request.GetResponse().AsString() == "The quick brown fox jumps over the lazy dog" );
+ CHECK( wxString(request.GetResponse().AsString().c_str()) == "The quick brown fox jumps over the lazy dog" );
The go-httpbin response ends with two \0 characters. See this response.
Converting it to a wxString and back removes these.
I don't want to add the \0 to the string on the right, to keep compatibility with normal httpbin.
> @@ -263,7 +263,7 @@ TEST_CASE_METHOD(RequestFixture,
pos += strlen(expectedKey);
// There may, or not, be a space after it.
- while ( wxIsspace(response[pos]) )
+ while ( wxIsspace(response[pos]) || response[pos] == '[' )
The go-httpbin response returns an array with values instead of just the value. See this response.
I'll update the comment above it.
In appveyor.yml:
> @@ -86,15 +88,21 @@ before_build: build_script: c:\projects\wxwidgets\build\tools\appveyor.bat before_test: +- cmd: | + echo Getting and launching httpbin. + rmdir %GOROOT% /s /q + mkdir %GOROOT% + appveyor DownloadFile https://golang.org/dl/go%GOVERSION%.windows-amd64.msi
Correct. IIRC go-httpbin requires at least 1.16. Most images have 1.13 installed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
You are receiving this because you are subscribed to this thread.
VZ
It looks like something got broken again in Python ecosystem. Can we try switching to this one?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Sure. I think I addressed all the comments, though not marked them as resolved yet.
I can rebase to check if it still works. But if you want to play with it, I don't mind if you push to this PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
If you could please rebase it, please do, otherwise I will, but I won't be able to do it today and maybe not tomorrow neither, unfortunately, and in the meanwhile Windows CMake CI seems to be permanently broken (i.e. the error above is not just a fluke, I've already tried rerunning the build and it failed again). TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 3a99995 Update go-httpbin
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks a lot! I've (squash) merged this now, let's see if it works better in the long run.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
It does still have some warnings in the action logs: Restore cache failed: Dependencies file is not found in /__w/wxWidgets/wxWidgets. Supported file pattern: go.sum
I think we should add this to all the setup-go:
with: cache: false
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks, done now.
There is also a lot of output like this when running the tests:
time="2024-03-13T13:18:02.9718" status=200 method="POST" uri="/post" size_bytes=604 duration_ms=0.08 user_agent="test/1 wxWidgets/3.3.0" client_ip=127.0.0.1:34276
Can they be avoided/redirected and shown only in case of error?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
I think something goes wrong with piping the output in https://github.com/wxWidgets/wxWidgets/blob/b03c3416c87e799cdd086be49761913e994451bd/build/tools/httpbin.sh#L13
I think go-httpbin -host 127.0.0.1 -port 8081 &> httpbin.log & should fix it (from https://askubuntu.com/a/731237 )
When there is an error, httpbin.log contents are already shown.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
We'd need to switch to using bash as shell in order to use &> which is a non-POSIX construct AFAIR and 2>&1 > should really work in the same way (as it means "redirect stderr to stdout and then redirect stdout to file") unless Go does something really weird. But then it just might, of course...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.