ci: run the no-GPU path on Windows - #160
Merged
Merged
Conversation
Probe commit: the job is not in required-green yet, because nothing has ever run this suite on Windows and the point is to find out what falls over. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
The toolchain file's components did not arrive with the channel on this runner; rustc-dev and llvm-tools are both published for x86_64-pc-windows-msvc at this nightly, so they just have to be asked for. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
The Windows job found it, and the probe was wrong in both directions. It looked in <sysroot>/lib for a name starting with librustc_driver. False negative, everywhere: librustc_driver-<hash>.so is in <sysroot>/lib on ANY toolchain, because it is the shared library rustc itself links -- it ships with the compiler, not with rustc-dev. A stable 1.98.0 with no rustc-dev has one match there and zero in lib/rustlib/<host>/lib, so the guard never fired for the case it exists for. False positive on Windows: windows-msvc puts rustc_driver-<hash>.dll under <sysroot>/bin, so <sysroot>/lib held nothing and a toolchain with rustc-dev correctly installed was reported as missing it -- the build stopped, which is exactly what the module docs promise not to do. It now looks for librustc_driver-<hash>.rmeta in <sysroot>/lib/rustlib/<host>/lib, where the component installs on every platform, and .rmeta because that is the metadata a dependent crate needs -- which is what this build is about to be. The prefix match excludes librustc_driver_impl-, which would otherwise pass. Checked both ways by running the probe standalone against two sysroots: silent on the nightly, and firing with the right path on stable. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
check_render clears the environment and puts five toolchain variables back, which is what makes the warm run and the PTY run agree on cargo's fingerprint. On Windows that set is missing SystemRoot, without which the platform's own networking DLLs cannot initialise -- so cargo failed with [6] Could not resolve hostname (Could not resolve host: index.crates.io) twice in a row, which reads exactly like a runner with no network and is not. The same clear also took PATHEXT, TEMP and USERPROFILE. The Windows names are behind cfg(windows) and the shared five are unchanged, so nothing about the Linux and macOS runs moves. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
The first Windows CI run found this: `cargo reconverge learn` panicked
there, for all four lessons.
The prose arrives through include_str!, which embeds the working tree's
bytes. A Windows checkout with core.autocrlf=true -- the default on a
windows-latest runner and on most Windows clones -- gives \r\n---\r\n,
so split("\n---\n") finds no separator, every lesson collapses to one
page, and the page-metadata assertion fires. A surviving \r would also
have rendered as a glyph.
pages() normalizes line endings before splitting, and the regression test
is written against the bytes rather than the platform, because a Linux
clone of a repository whose files were committed with CRLF has the same
problem. Removing the normalization fails it.
.gitattributes pins the tree to LF as well. The code no longer needs it,
but the golden and snapshot files the PTY suites compare byte-for-byte
still would.
Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
The Windows job failed a third time, on the watch test, with error: could not compile `proc-macro2` (build script) which is the same cause as the second failure wearing different clothes: triage_cli.rs clears the environment too, and had its own inline list -- twice -- written on Linux. Without TEMP a build script has nowhere to work, as without SystemRoot the resolver cannot resolve. Three copies of that list is how the first fix missed two of the three places, so there is one now, in tests/common/mod.rs, with the reason written down beside it. check_render keeps the same behaviour and gains RUSTUP_TOOLCHAIN and RUSTC, which triage_cli had already learned it needed: without the pin the child's cargo and rustc can resolve differently from the caller's. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
Three real bugs in, the job is worth requiring. What it cannot run is one suite: emulation waits on a complete frame and ConPTY does not deliver frame boundaries the way the other two consoles do -- the screen in the failure is the right screen, fully painted. Quarantined with the reason and #162 rather than skipped, per CONTRIBUTING section 3, because the reduction that would go upstream needs a Windows box this change did not have. Every other PTY suite here passes on Windows. The README's Limitations section now states the supported platforms and those two exceptions, which is the thing that was missing: not the platform, the silence about it. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
The per-test quarantine was the wrong shape. It is not two tests: under ConPTY the PTY suites time out waiting for a frame boundary while the screen they print is the right screen, fully painted, and emulation and inspect_flow both show it. That is how the console delivers what an application wrote, not what these views render. So the Windows job runs the workspace minus the TUI crate's PTY suites, plus that crate's own state and rendering unit tests -- which is everything the README claims runs anywhere: the analysis, check, the artifact round-trip, the schemas and the CLI contract. The PTY suites keep their Linux and macOS legs, which is where they have always been the terminal gate. README and CHANGELOG say so plainly rather than implying the views are gated on three platforms. #162 holds the root-cause work; it needs a Windows machine to produce the reduction that would go upstream. Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
Signed-off-by: Vyncint Ng <chivy.nguyen@manabie.com>
vyncint
force-pushed
the
ci/windows-no-gpu-path
branch
from
September 22, 2026 07:21
6fcd22a to
1b76b40
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Probing what actually breaks. Draft body; will be rewritten once the job reports.