Pull requests / #1658

#1658 tools tests: align test_setup_risk's WSL mock with install() (the KV-streaming rule test failed inside WSL)

open · @chaog992 · 0 comments · View on GitHub

Setup & installAMD / HIPNVIDIA / CUDAModels & quantsWindowsLinux

Description

## Title
Issue: none opened. The same kind of failure as #974 (fixed test-only in d1ec32b), here in `tools/test_setup_risk.py`.

## Summary
Inside WSL, one test in `tools/test_setup_risk.py` fails on main (fb58e0d):
`RulesAgreeWithTheWrittenConfig.test_the_streaming_rule_is_the_flag_setup_writes`. The test asked
`kv_streaming_wanted()` on a different PC from the one `setup.main()` ran on. This change runs the whole test on
the same mocked PC. It changes only the test, by one line.

## What changed
`tools/test_setup_risk.py` (+1):
- The test method now has `@mock.patch.object(setup, "is_wsl", lambda: False)`, the mock `install()` already
  applies. So `kv_streaming_wanted()` is asked on the same PC as `setup.main()`.
- The assertion is unchanged.
- d1ec32b fixed the unsloth tests for #974 with the same mock.

Why it failed: `setup.main()` runs inside `install()`, which mocks `is_wsl()` to False
(`tools/test_setup_golden.py` line 106). The test then called `setup.kv_streaming_wanted()` outside that mock.
Inside WSL the real `is_wsl()` returns True, so the rule said "no streaming" while the config that setup wrote for the
mocked PC has `--kv-resident 32768`. The test failed at its first 131072 case (setup's args shortened):

```
AssertionError: True != False : Q2_0 at 131072 with int8, --kv-streaming auto: setup wrote [..., '--kv-resident', '32768']
```

In this test's 54 cases, setup's decision is not at fault. When `is_wsl()` is the same on both sides, setup and the
rule agree in all of them:
- False on both sides: the test passes on fb58e0d.
- True on both sides: all 54 match, and setup writes `--kv-resident` in none of them.

Left alone:
- `setup.py`, so what setup writes is unchanged.
- The class's low-RAM test, because `low_ram_wanted` does not read `is_wsl`.
- Every other file. No CUDA, HIP or SYCL code is touched.

This is independent of #1498, which changes only `src/core/layer.cpp`.

## Extra Notes
Checked on WSL2 (Ubuntu 24.04.1, kernel 5.15.167.4) with Python 3.12.3. Base fb58e0d, head 4a9c115.

- `python3 tools/test_setup_risk.py`: FAILED (failures=1) on the base, Ran 40 tests, OK on the head.
  `python3 -m unittest tools.test_setup_risk` also passes on the head.
- After the test, whether it passes or fails, `setup.is_wsl` is the original function again.
- The test still catches the two copies of the rule disagreeing:
  - with the 1 GB of RAM headroom raised to 100 GB only in `kv_streaming_wanted()`, it fails (`True != False`);
  - with it raised only in `setup.main()`'s `stream_fits`, it fails (`False != True`).
- With `is_wsl` forced to False around the test, as on Windows or native Linux, it passes on the base and the
  head, so nothing changes off WSL.
- With `is_wsl` forced to True, it fails on the base and passes on the head. To reproduce off WSL, run this from
  the checkout root:

  ```
  python3 -c "import sys, unittest; from unittest import mock; sys.path[:0] = ['.', 'tools']; import setup, test_setup_risk as t; mock.patch.object(setup, 'is_wsl', lambda: True).start(); unittest.main(module=t, argv=['t', 'RulesAgreeWithTheWrittenConfig.test_the_streaming_rule_is_the_flag_setup_writes'])"
  ```

- The other 27 `tools/test_setup_*.py` files give the same results on the base and the head. Two of them fail on
  this machine for other reasons, on both, and this PR does not touch them: `test_setup_engine_hash` (#1510) and
  `test_setup_unsloth`'s `test_amd_is_not_asked`.
- Not run: Windows and native Linux (only simulated with the `is_wsl` patch above), and other Python versions.
- In the repo's Python files, no test patches `is_wsl` to anything but False, and none touches `platform.uname`.
  So setup's WSL branches have no test of their own. Adding one would be a separate change.

Related on strata.com

Editorial links to help you install, pick models, or read release notes — not part of the upstream thread.