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

chore: remove server.host option for e2e test #14606

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

Dunqing
Copy link
Contributor

@Dunqing Dunqing commented Oct 12, 2023

Description

An edge case I encountered. Ports 5173 are occupied by another vite server that uses the server.host option by default. When I run pnpm test-serve, the test server will listen() successfully. Normally it should fail and then start port 5174 but it doesn't.

I noticed server.host = true in e2e test, so the host is undefined. According to the documentation described as follows

If host is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise.

Learn more about node documentation

So the http server may not listen to localhost. maybe 192.168.xx.xx or else.

The solution to this issue is easy, just use network[0] instead of local[0] will work

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@chaejunlee
Copy link
Contributor

I came across this problem quite a few times, too! Glad to see that it can be fixed!

@bluwy
Copy link
Member

bluwy commented Oct 13, 2023

Maybe we can remove the host: true config altogether? Seems to be added in #2977 but even if I removed it, it still seem to work.

@Dunqing
Copy link
Contributor Author

Dunqing commented Oct 13, 2023

Maybe we can remove the host: true config altogether? Seems to be added in #2977 but even if I removed it, it still seem to work.

Yes. I'm fine with either solution.

@patak-dev
Copy link
Member

I also vote to remove host here 👍🏼

@Dunqing Dunqing changed the title chore: update vite test url for e2e chore: remove server.host option for e2e test Oct 13, 2023
@sapphi-red sapphi-red merged commit f2aadb0 into vitejs:main Oct 14, 2023
11 checks passed
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

5 participants