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

FsockopenTest: add regression test for HTTP string #520

Closed
wants to merge 2 commits into from

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 30, 2021

PR #339 made a change to safeguard that the HTTP string would always contain a decimal point, never a comma.

The original bug was related to the current locale influencing the results of the call to sprintf().

At the time, no test was added to safeguard the fix against regressions.

This PR now adds this regression test.

I've verified that without the fix as merged in #339, this test would fail.

Includes adjusting the GH Actions workflows to install extra locales as otherwise the test would fail on the locale being changed to in the test not being available.

PR 339 made a change to safeguard that the HTTP string would always contain a decimal point, never a comma.

The original bug was related to the current locale influencing the results of the call to `sprintf()`.

At the time, no test was added to safeguard the fix against regressions.

This PR now adds this regression test.

I've verified that without the fix as merged in 339, this test would fail.
@jrfnl jrfnl force-pushed the feature/472-add-http-version-string-test branch from abc0fc1 to 9870f51 Compare August 30, 2021 05:17
@jrfnl
Copy link
Member Author

jrfnl commented Aug 30, 2021

Closing for the time being while debugging why the locale change is not working on GHA.

@jrfnl jrfnl closed this Aug 30, 2021
@jrfnl
Copy link
Member Author

jrfnl commented Aug 30, 2021

The locale change needed a tweak to the GH Actions workflows. Re-opening the PR now this has been fixed.

@jrfnl
Copy link
Member Author

jrfnl commented Aug 30, 2021

Oh .. great.. can't reopen once there's been a force-push. Oh well. Opening a new PR instead in that case. See #521

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant