Skip to content

ci: run the -prod test suite on Windows as well - #534

Closed
metif12 wants to merge 1 commit into
vlang:masterfrom
metif12:ci/run-prod-tests-on-windows
Closed

metif12 wants to merge 1 commit into
vlang:masterfrom
metif12:ci/run-prod-tests-on-windows

Conversation

@metif12

@metif12 metif12 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Problem

-prod removes assert statements, and that silently changes the behaviour of
any test that performs work inside one. In this repository it reached Windows as a
hang rather than a failure. Two tests did:

assert os.fd_dup2(transport.read_fd, 0) >= 0

The dup2 there is the operation the test depends on, not a check on it. Under
-prod the statement disappears, descriptor 0 keeps pointing at the real stdin,
and the read that follows blocks forever:

v -prod test lsp_test.v          -> never terminated (>40 min, vs 25s)
v -prod test integration_test.v -> likewise
v -prod test .                   -> never completed

Nothing caught it, because the -prod step only ran on Linux:

- name: Run tests with production optimizations
  if: runner.os == 'Linux'

Fix

1. Run the same step on Windows.

- name: Run tests with production optimizations (Windows)
  if: runner.os == 'Windows'
  timeout-minutes: 30
  env:
    V_MACOS_V3_NO_FALLBACK: "0"
  run: v -no-memory-limit -nocache -prod test vls/

"0" matches the existing Windows test step, since Windows defaults to V1 and 0
permits fallback.

2. Bound both -prod steps with timeout-minutes: 30. A hang in a test step
otherwise consumes the job's entire budget and reports nothing useful. A bounded
step turns it into a red build naming the step that stopped. The Linux step gets
the same guard because it has the same failure mode — drop that one line if you
would rather keep the diff Windows-only.

The Windows step deliberately does not set VLS_VLANG_V_REPO. That env var
enables the slow vlang/v workspace tests, which are already covered on Windows by
the non--prod step above, which does set it. Leaving it off keeps the added CI
time to roughly the 1-2 minutes the suite takes locally under -prod, instead of
the much longer run those tests add on top of -prod.

Validation

V 0137eb5 (the vlang/v revision CI builds from), Windows:

v -no-memory-limit -nocache -prod test .    ->  completes in 76s

76 seconds unbounded versus never terminating before #532. The 30-minute budget
is deliberately generous headroom over that.

I could not get a trustworthy figure for -prod combined with the vlang/v
workspace tests on this machine: another V build was running a full test .
concurrently and the two contended for all cores, so the number I got (over 50
minutes) reflects the contention, not the workload. That is exactly why the step
does not enable those tests.

Ordering constraint — please read before merging

This must land after #532. Before
that fix the two stdio tests above hang under -prod on Windows, so this step
goes red. With the timeout it fails in a bounded, legible way rather than
hanging, but it is still red.

The Windows job also needs #526 (the
suite must compile) and #528 (the job
currently dies at Check formatting) before it can be green at all.

Practical order: #526, #527, #528, #532, then this.

Related

#533 closes the matching codegen gap —
V3 was never compiled on Windows either — and has no ordering dependency.

`-prod` removes assert statements, and that silently changes the behaviour of any
test that performs work inside one. In this repository that reached Windows as a
hang rather than a failure: `test_stdio_reader_processes_frame_before_eof` and
`test_integration_stdio_initialize_completion_and_hover` both did

    assert os.fd_dup2(transport.read_fd, 0) >= 0

so under `-prod` the descriptor was never redirected and the following read
blocked forever. Nothing caught it because the `-prod` step was gated to Linux:

    - name: Run tests with production optimizations
      if: runner.os == 'Linux'

Two changes:

1. Add the same step for Windows. `V_MACOS_V3_NO_FALLBACK` is "0" to match the
   existing Windows test step, since Windows defaults to V1 and 0 permits
   fallback.

2. Put `timeout-minutes: 30` on both. A hang in a test step otherwise consumes the
   job's entire budget and reports nothing useful; a bounded step turns it into
   a red build that says which step stopped. The Linux step gets the same guard
   because it has the same failure mode. Drop that one line if you would rather
   keep the diff to Windows only.

The Windows `-prod` step deliberately does not set `VLS_VLANG_V_REPO`, so it does
not repeat the slow vlang/v workspace tests. Those are already covered on Windows
by the non-prod step above, which does set it. That also keeps the added CI time
to the ~1-2 minutes the suite takes locally under `-prod`, instead of the
considerably longer run the workspace tests add on top of `-prod`.

Verified on Windows with V 0137eb5:

    v -no-memory-limit -nocache -prod test vls/   ->  completes in 76s

**Ordering: this must land after vlang#532.** Before that fix the two stdio tests
above hang under `-prod` on Windows, so this step goes red. With the timeout in
place it fails in a bounded, legible way instead of hanging, but it is still red.
The Windows job also needs vlang#526 (the suite must compile) and vlang#528 (the job
currently dies at `Check formatting`) to be green at all.

Practical merge order: vlang#526, vlang#527, vlang#528, vlang#532, then this.
@metif12

metif12 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Consolidated rather than merged on its own.

Folded into #536, which also carries #535's commits because master cannot build a green
suite without them. Once #535 lands, what remains here is this change.

Please review #536 instead. Nothing here is lost.

@metif12

metif12 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Closed in favour of #536: run the -prod suite on Windows.

@metif12 metif12 closed this Oct 5, 2026
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.

1 participant