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

test: fix test failure due to localhost being also ipv6 #845

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

ikonst
Copy link
Contributor

@ikonst ikonst commented Oct 28, 2024

On my MacOS machine:

% host localhost.   
localhost has address 127.0.0.1
localhost has IPv6 address ::1

This caused the "should handle connection error" test to fail, since the error was an AggregateError containing both the 127.0.0.1 and the ::1 failure.

To make this uniform across different test environments, I'm changing the hostname to an IPv4 IP address. (It might be worthwhile to test AggregateError behavior and IPv6 behavior but this is not the goal of this PR.)

Checklist

  • I have ensured my pull request is not behind the main or master branch of the original repository.
  • I have rebased all commits where necessary so that reviewing this pull request can be done without having to merge it first.
  • I have written a commit message that passes commitlint linting.
  • I have ensured that my code changes pass linting tests.
  • I have ensured that my code changes pass unit tests.
  • I have described my pull request and the reasons for code changes along with context if necessary.

Sorry, something went wrong.

@ikonst ikonst changed the title tests: fix test failure due to localhost being also ipv6 test: fix test failure due to localhost being also ipv6 Oct 28, 2024
Depending on runtime environment, "localhost" might resolve to both IPv4 and IPv6, resulting in an AggregateError in place of an Error.
@ikonst
Copy link
Contributor Author

ikonst commented Oct 28, 2024

👋 @titanism

@ikonst
Copy link
Contributor Author

ikonst commented Nov 6, 2024

@titanism could you take a look?

@titanism titanism merged commit be0d5c7 into ladjs:master Mar 20, 2025
2 checks passed
@titanism
Copy link
Collaborator

v7.1.0 released to npm – please try it out and let us know if it works for you 🎉

https://github.com/ladjs/supertest/releases/tag/v7.1.0

Thank you for your contributions 🙏

Our efforts to maintain this project are through our work on https://forwardemail.net (@forwardemail)

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.

None yet

2 participants