From be6b07e6a159115795c70f677495b1048b6ff4e1 Mon Sep 17 00:00:00 2001 From: Karin Hendrikse <30577427+khendrikse@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:05:31 +0100 Subject: [PATCH] fix: check ip version if settings.useStaticServer (#6936) * fix: check ip version if settings.useStaticServer * chore: different way of getting ip version * chore: run tests on win and node23 * chore: let dev test check for errors not warnings * chore: fix cleanup for copy-template-dir test * chore: fix cleanup for copy-template-dir test again, revert * test: add single test for dev command for windows latest using node 23 * test: will test fail without new code * test: fix code * fix: add comments and remove matrix --- .github/workflows/integration-tests.yml | 98 ++++++++++++++++++++++ src/utils/framework-server.ts | 5 +- src/utils/static-server.ts | 3 +- tests/integration/commands/dev/dev.test.ts | 5 +- 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index ba186ba645a..4b24667ea85 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -123,3 +123,101 @@ jobs: flags: ${{ steps.test-coverage-flags.outputs.os }},${{ steps.test-coverage-flags.outputs.node }} token: ${{ secrets.CODECOV_TOKEN }} if: '${{ !steps.release-check.outputs.IS_RELEASE }}' + # Specific tests for known test that failed on windows using node 23. + # Can be replaced with larger node 23 tests in the future. + integration-win-node-23: + name: Integration test windows latest node23 specific + runs-on: windows-latest + timeout-minutes: 40 + steps: + # Sets an output parameter if this is a release PR + - name: Check for release + id: release-check + # For windows we have to use $env: + run: |- + echo "IS_RELEASE=true" >> $GITHUB_OUTPUT + echo "IS_RELEASE=true" >> $env:GITHUB_OUTPUT + if: "${{ startsWith(github.head_ref, 'release-') }}" + # This improves Windows network performance, we need this since we open many ports in our tests + - name: Increase Windows port limit and reduce time wait delay + run: | + netsh int ipv4 set dynamicport tcp start=1025 num=64511 + REG ADD HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\TCPIP\Parameters /v TcpTimedWaitDelay /t REG_DWORD /d 30 /f + - name: Git checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + - name: Use Node.js + uses: actions/setup-node@v4 + with: + node-version: '23.x' + cache: npm + check-latest: true + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + - name: Install PNPM + run: | + corepack enable + corepack prepare pnpm@9.14.2 --activate + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + - name: Setup Deno + uses: denoland/setup-deno@v1 + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + with: + deno-version: v1.44.4 + - name: Install core dependencies + run: npm ci --no-audit + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + - name: Generate self-signed certificates + run: npm run certs + if: '${{!steps.release-check.outputs.IS_RELEASE}}' + shell: bash + - name: Prepare tests + run: npm run test:init + if: '${{ !steps.release-check.outputs.IS_RELEASE }}' + - name: Tests + uses: nick-fields/retry@v3 + if: '${{ !steps.release-check.outputs.IS_RELEASE }}' + with: + timeout_minutes: 15 + max_attempts: 3 + retry_on: error + command: npm exec vitest -- run tests/integration/commands/dev/dev.test.ts + env: + # GitHub secrets are not available when running on PR from forks + # We set a flag so we can skip tests that access Netlify API + NETLIFY_TEST_DISABLE_LIVE: + ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true }} + NETLIFY_AUTH_TOKEN: ${{ secrets.NETLIFY_AUTH_TOKEN }} + # NETLIFY_TEST_GITHUB_TOKEN is used to avoid reaching GitHub API limits in exec-fetcher.js + NETLIFY_TEST_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Changes the polling interval used by the file watcher + CHOKIDAR_INTERVAL: 20 + CHOKIDAR_USEPOLLING: 1 + - name: Get test coverage flags + id: test-coverage-flags + # For windows we have to use $env: + run: |- + os=windows-latest + node=$(node --version) + echo "os=${os/-latest/}" >> $GITHUB_OUTPUT + echo "os=${os/-latest/}" >> $env:GITHUB_OUTPUT + echo "node=node_${node/.*.*/}" >> $GITHUB_OUTPUT + echo "node=node_${node/.*.*/}" >> $env:GITHUB_OUTPUT + shell: bash + if: '${{ !steps.release-check.outputs.IS_RELEASE }}' + + - name: Store npm error artefacts + uses: actions/upload-artifact@v4 + if: always() + with: + name: npm-logs--windows-latest--23x + path: | + /home/runner/.npm/_logs/**/* + + - uses: codecov/codecov-action@v4 + continue-on-error: true + with: + flags: ${{ steps.test-coverage-flags.outputs.os }},${{ steps.test-coverage-flags.outputs.node }} + token: ${{ secrets.CODECOV_TOKEN }} + if: '${{ !steps.release-check.outputs.IS_RELEASE }}' diff --git a/src/utils/framework-server.ts b/src/utils/framework-server.ts index b7652a806e2..857e51c5863 100644 --- a/src/utils/framework-server.ts +++ b/src/utils/framework-server.ts @@ -30,9 +30,8 @@ export const startFrameworkServer = async function ({ if (settings.command) { runCommand(settings.command, { env: settings.env, cwd }) } - await startStaticServer({ settings }) - - return {} + const { family } = await startStaticServer({ settings }) + return { ipVersion: family === 'IPv6' ? 6 : 4 } } log(`${NETLIFYDEVLOG} Starting Netlify Dev with ${settings.framework || 'custom config'}`) diff --git a/src/utils/static-server.ts b/src/utils/static-server.ts index b9f4a348255..87a73ade016 100644 --- a/src/utils/static-server.ts +++ b/src/utils/static-server.ts @@ -33,7 +33,8 @@ export const startStaticServer = async ({ settings }) => { } done() }) - await server.listen({ port: settings.frameworkPort }) + const [address] = server.addresses() log(`\n${NETLIFYDEVLOG} Static server listening to`, settings.frameworkPort) + return { family: address.family } } diff --git a/tests/integration/commands/dev/dev.test.ts b/tests/integration/commands/dev/dev.test.ts index 95756d5f190..2045014d04e 100644 --- a/tests/integration/commands/dev/dev.test.ts +++ b/tests/integration/commands/dev/dev.test.ts @@ -682,7 +682,10 @@ describe.concurrent('command/dev', () => { async (server) => { const output = server.outputBuffer.map((buff) => buff.toString()).join('\n') t.expect(output).toContain('Server now ready') - t.expect(server.errorBuffer).toHaveLength(0) + // With node 23 we might be getting some warnings from one of our dependencies + // which should go away once this is merged: https://github.com/debug-js/debug/pull/977 + const errorOutput = server.errorBuffer.map((buff) => buff.toString()).join('\n') + t.expect(errorOutput).not.toContain('Error') }, ) })