pytorch / pytorch/pytorch

CI Not Detecting Failing Tests in test/distributed/elastic/*

Open
#167,994 1 comment 0 reactions 0 assignees View on GitHub
bot-triaged module: ci oncall: distributed oncall: distributed infra ptd-bot-triaged triaged
Dominant language
Python
Stars
103k
Forks
29.5k
PR merge metrics
PR metrics pending

Description

A significant number of tests under `test/distributed/elastic/` are failing, but CI does **not** surface these failures, possibly same with test/distributed/launcher, Many of these tests appear to have been broken for a long time without detection. I opened a PR with fixes, but I believe this warrants an issue so the team can investigate why CI is not catching failures in this directory.

PR with fixes: https://github.com/pytorch/pytorch/pull/167993

### `test/distributed/elastic/rendezvous/c10d_rendezvous_backend_test.py`

**Issue:**
In `test_create_backend_returns_backend_if_is_host_is_false` and
`test_create_backend_returns_backend_if_is_not_specified_and_store_already_exists`, commit https://github.com/pytorch/pytorch/commit/d25e6e623fea0552d1a4b3124344d1b2c499f6f8 removed the unused `store` variable. This caused the `TCPStore` to be garbage-collected immediately, and the tests fail as a result.

---

### `test/distributed/elastic/rendezvous/dynamic_rendezvous_test.py`

#### Issue 1
`datetime.utcnow` was replaced with `datetime.now` in the implementation PR https://github.com/pytorch/pytorch/pull/136141, but the tests were not updated.

#### Issue 2
PR https://github.com/pytorch/pytorch/pull/145228 changed `create_handler()` to expect `keep_alive_interval` as an `int`, but the test `test_redundancy_transition_to_wait_list_then_join_rendezvous` passes `timedelta(seconds=1)`.

#### Issue 3
`test_share_tcp_store_from_backend` mocks `dist.PrefixStore` but also calls
`CustomPrefixStore(spec=dist.PrefixStore)`. Since `dist.PrefixStore` is already patched, this results in:

> Cannot spec a Mock object

---

### `test/distributed/elastic/rendezvous/etcd_server_test.py`

**Issue:**
In `test_etcd_server_with_rendezvous`, the `EtcdRendezvous` prefix does not include a leading slash, but etcd v2 always stores keys with one. This causes a hang during the `rdzv_handler.next_rendezvous()` → `RendezvousStoreInfo.build` → (`store.set` → `store.get`), because the key is written as `test/run_1/rdzv/v_1/kv/TUFTVEVSX0FERFI=` but etcd stores it as `/test/run_1/rdzv/v_1/kv/TUFTVEVSX0FERFI=`. Since `store.get` (via `ETCDStore._try_wait_get`) looks for the non–slash-prefixed key, it never finds

---

### `test/distributed/elastic/rendezvous/out_of_tree_rendezvous_test.py`

**Issue:**
`test_out_of_tree_handler_loading` attempts to test out-of-tree handler registration by adding a directory to `sys.path`.
However, the real mechanism uses Python entry points, which require pip installation.
The original PR https://github.com/pytorch/pytorch/pull/132633 used pip install, but after review it was replaced with `sys.path` modification — which probably only worked locally due to stale installations.

---

### `torch/distributed/elastic/rendezvous/etcd_rendezvous.py`

**Issue:**
PR https://github.com/pytorch/pytorch/pull/135262 added an optional `local_addr` parameter to `EtcdRendezvousHandler.__init__`, but did not define a default value.
This breaks `test_etcd_server_with_rendezvous` in `test/distributed/elastic/rendezvous/etcd_server_test.py`

---

Following test might actually be passing in some environments and configs.
### `test/distributed/launcher/test_run.py`

**Issue:**
`nproc_type="auto"` determines world size using `torch.accelerator.is_available()`, but the test incorrectly patches `torch.cuda.is_available()`.

cc @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @pragupta @msaroufim @dcci @aditvenk @weifengpy @malfet @pytorch/pytorch-dev-infra @seemethere @H-Huang

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.