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: In rest_server_test Allow "view-localhost" hostname #1282

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

dgreatwood
Copy link
Collaborator

The test already allowed "localhost" and "ip6-localhost". "view-localhost" was seen on Windows 10 by @Kreijstal . The test now allows any hostname ending in "localhost" to be accepted as a hostname.

Addresses part of issue #1281.

The test already allowed "localhost" and "ip6-localhost".
"view-localhost" was seen on Windows 10 by @Kreijstal . We now allow
any hostname ending in "localhost" to be accepted as a hostname.

Addresses part of issue pistacheio#1281.
@kiplingw kiplingw added bug portability unit / CI testing For unit testing and CI (including DEP-8) Windows labels Jan 14, 2025
@dgreatwood dgreatwood merged commit 5798cc7 into pistacheio:master Jan 14, 2025
134 of 135 checks passed
@@ -117,14 +117,14 @@ TEST(rest_server_test, basic_test)
ASSERT_EQ(res->status, 200);

// TODO: Clean this up to use proper gtest macros.
// NOTE: res->body is "ip6-localhost" on some architectures.
if (res->body == "ip6-localhost")
if ((res->body.length() >= 9) && // 9 being len of "localhost"
Copy link
Member

Choose a reason for hiding this comment

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

instead of hardcoding 9, you could write strlen("localhost"). It's self-documenting

Copy link
Member

Choose a reason for hiding this comment

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

Or, even better, you can use EXPECT_THAT(res->body, testing::EndsWith("localhost"))

@dgreatwood dgreatwood deleted the restSrvTestViewHostname branch January 18, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug portability unit / CI testing For unit testing and CI (including DEP-8) Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants