test: withr-free local seed and RNG-state leak fixes by schochastics · Pull Request #2714 · igraph/rigraph

schochastics · 2026-06-25T14:45:01Z

What & why

The test suite has an opt-in check (gated behind IGRAPH_CHECK_RNG_STATE=true in
tests/testthat/setup.R) that fails any test which leaves the global
.Random.seed changed. This PR makes the whole suite pass that check and moves
the suite's RNG-seed handling off withr onto in-house helpers.

Running the suite with IGRAPH_CHECK_RNG_STATE=true: 0 seed leaks, 0
failures, 0 warnings (9739 passing).

New helpers (`tests/testthat/helper.R`)

igraph_local_seed() — drop-in for withr::local_seed(). Sets the seed for
the current scope and restores the previous global RNG state when that scope
exits.

Self-contained: snapshots and restores RNGkind + .Random.seed itself
(uses withr::defer only as the deferral primitive), so it doesn't inherit
withr::local_seed's past seed-handling bugs.
Restores RNGkind first, then .Random.seed last — changing the kind
discards the seed, so the seed write must come last for an exact round-trip.
Optional rng_version (e.g. "3.5.0"), folding in what used to be a separate
local_rng_version() call. Pairing two independent deferred restores (kind +
seed) ran them in the wrong order and leaked; a single ordered restore fixes it.

igraph_with_seed() — block form mirroring withr::with_seed(): set the
seed, evaluate the block, restore. Needed where a single test has several
independently seeded blocks with code between them (e.g. test-print-classic.R),
which a frame-scoped igraph_local_seed() can't express.

setup.R

Set a baseline seed (set.seed(42)) so .Random.seed always exists before
tests run. Otherwise the first RNG-touching test creates it from nothing
(many igraph C functions initialise the RNG without drawing from it, e.g.
voronoi_cells()), which the check flagged — and restoring that absent state
just shoved the "first creation" onto the next test (whack-a-mole).
The seed-leak override now forwards the test block via
substitute() + eval(as.call(...)) instead of
rlang::inject(testthat::test_that(name, !!code)). inject() eagerly
processed any injection operator in the test body (!!, !!!, {{),
e.g. set_vertex_attrs(g, !!!attr_list), evaluating it in the wrong frame and
erroring with object 'attr_list' not found.
The check stays gated behind IGRAPH_CHECK_RNG_STATE=true (opt-in for CI).

Test files

Moved helper functions that were defined inside test files into a shared
helper-test-functions.R (the override re-scopes test code, so file-local
helpers were no longer reachable from the test body).
Added igraph_local_seed() to tests that genuinely advance the RNG (unseeded
sample_* fixtures and ARPACK eigensolvers).
Replaced all 875 withr::local_seed() call sites with
igraph_local_seed() (including the 732 in test-aaa-auto.R).
Replaced the 9 withr::with_seed() call sites with igraph_with_seed()
(test-print-classic.R, test-layout.R, test-iterators.R). These never
leaked — consistency cleanup; snapshots unchanged.
Replaced every local_rng_version("3.5.0") + seed pairing with
igraph_local_seed(seed, rng_version = "3.5.0"), then removed the now-unused
local_rng_version() helper.
restore_rng_state() suppresses warnings around the RNGkind restore so
putting back the older "Rounding" sampler (RNGversion("3.5.0")) doesn't
emit R's "non-uniform 'Rounding' sampler used" note.

tools/AGENTS.md

Updated the test-writing guidance to use igraph_local_seed, so newly added
test-aaa-auto.R tests follow the convention.

Notes

igraph_local_seed()/igraph_with_seed() live in helper.R (not setup.R)
so helper functions like make_scan_graphs() can see them.
The suite no longer uses withr for any RNG-seed handling.

🤖 Generated with Claude Code

The test_that override in setup.R fails any test that changes the global .Random.seed (gated behind IGRAPH_CHECK_RNG_STATE). Several things had to change for the suite to pass under it: * Move helper functions defined inside test files into a shared helper-test-functions.R. The override re-scopes test code, so file-local helpers were no longer reachable from the test body. * Add igraph_local_seed() (helper.R): a withr::local_seed-free local seed that snapshots and restores RNGkind + .Random.seed itself, restoring the kind first and the seed last so the state round-trips exactly. It takes an optional rng_version, replacing the leaky local_rng_version() + withr::local_seed() pairing (whose two independent deferred restores ran in the wrong order). * setup.R: set a baseline seed so .Random.seed always exists -- otherwise the first RNG-touching test creates it from nothing (many igraph C functions init the RNG without drawing, e.g. voronoi_cells()), which the check flagged. Also forward the test block via substitute()/eval() instead of rlang::inject(testthat::test_that(name, !!code)); inject() mangled test bodies containing !!/!!!/{{ (e.g. set_vertex_attrs(g, !!!attr_list)). * Add igraph_local_seed() to the tests that genuinely advance the RNG (unseeded sample_* fixtures and ARPACK eigensolvers), and convert the local_rng_version() + withr::local_seed() pairs to igraph_local_seed(rng_version = "3.5.0"). Full suite passes the check: 0 seed leaks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Migrate all 875 withr::local_seed() call sites in the test suite to igraph_local_seed(), completing the move off withr for RNG-seed handling. All calls passed only a seed, so this is a direct swap; igraph_local_seed() restores RNGkind + .Random.seed itself (a superset of withr::local_seed's behaviour). Also: * restore_rng_state() now suppresses warnings around the RNGkind restore, so putting back an older sampler (the "Rounding" sample.kind from RNGversion("3.5.0")) no longer emits R's "non-uniform 'Rounding' sampler used" note. This surfaced once the inner withr::local_seed calls nested under igraph_local_seed(rng_version = "3.5.0") were migrated; withr had suppressed it. * tools/AGENTS.md: update the test-writing guidance to use igraph_local_seed so newly added test-aaa-auto.R tests follow the convention. Full suite passes the seed-leak check with no warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…e checker

Add igraph_with_seed(), a block-form companion to igraph_local_seed() that mirrors withr::with_seed(): set the seed, evaluate the block, restore the prior global RNG state. It is needed where a single test has several independently seeded blocks with code between them (e.g. test-print-classic.R), which a frame-scoped igraph_local_seed() can't express. Migrate the 9 withr::with_seed() call sites (test-print-classic.R, test-layout.R, test-iterators.R) to igraph_with_seed(), completing the removal of withr from the suite's RNG-seed handling. These never leaked (with_seed is self-contained); this is consistency cleanup. Snapshots unchanged; suite passes the seed-leak check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Dead code: every local_rng_version("3.5.0") call was folded into igraph_local_seed(seed, rng_version = "3.5.0") earlier in this branch, leaving no call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maelle

Thank you!! 🎲

maelle · 2026-07-03T12:06:02Z

-  withr::defer(do.call(RNGkind, as.list(orig)), envir = .local_envir)
-  suppressWarnings(RNGversion(version))
-  orig
+# Restore the global RNG state (kind + .Random.seed) to a previously captured


Could the line breaks be after sentences? 💅

maelle · 2026-07-03T12:08:39Z

+  # sample.kind that RNGversion("3.5.0") selects) otherwise emits R's
+  # "non-uniform 'Rounding' sampler used" note. We're only putting back a state
+  # that was already in effect, so the note is noise.
+  suppressWarnings(do.call(RNGkind, as.list(kind)))


how I wish we could suppress that warning exactly (with a class?) and remove the comment.

maelle · 2026-07-03T12:10:12Z

+  }
+}
+
+# Drop-in replacement for withr::local_seed() in tests. Sets the RNG seed (and


put this function first, as it's the higher level one. (sort of https://style.tidyverse.org/package-files.html#organisation)

For instance when reviewing, it'd make more sense to see this function first.

maelle · 2026-07-03T12:10:41Z

+# optionally pins the RNG version via `rng_version`, e.g. "3.5.0") and restores
+# the pre-call global RNG state when `.local_envir` exits (the current test_that
+# block, or the enclosing helper function when called from one). Self-contained:
+# it does its own save/restore, so it does not depend on withr's local_seed().


link to some withr issue to explain our choice?

maelle · 2026-07-03T12:11:27Z

+# block, or the enclosing helper function when called from one). Self-contained:
+# it does its own save/restore, so it does not depend on withr's local_seed().
+# withr::defer is used only as the deferral primitive (it fires reliably in both
+# test blocks and function frames). Folding the version pin in here -- rather


I must say this is a complicated sentence for me. Maybe rephrase in two sentences with an example for each?

maelle · 2026-07-03T12:12:58Z

+    sample.kind = sample.kind
+  )
+
+  invisible(old_seed)


why? convention in withr?

maelle · 2026-07-03T12:13:59Z


+# Establish a baseline global RNG state so `.Random.seed` always exists before
+# any test runs. Without this, the first test to touch the RNG would *create*
+# `.Random.seed` from nothing (many igraph C functions initialise the RNG even


is that bad design in those functions?

maelle · 2026-07-03T12:14:37Z

+# does when `before` was NULL) just pushes the creation onto the next test -- a
+# whack-a-mole. Pinning a baseline here means only tests that genuinely *advance*
+# the seed need igraph_local_seed().
+set.seed(42)


and this seed never leaks in an user's interactive session?

No this doesn leak. Test run in there own R throwaway R process

maelle · 2026-07-03T12:15:53Z

+# so they are visible both to test code and to helper functions that call them.
+
 if (Sys.getenv("IGRAPH_CHECK_RNG_STATE") == "true") {
  # Check that tests don't change the random seed


I don't understand why we need this if we're confident in our solution.

And when will we set this env var? In a GHA workflow? Before each release?

There will be a specific runner that has this set to true (at least thats what Kirill was planing

maelle · 2026-07-03T12:16:38Z

  }
 }
+
+# NOTE (why the override forwards `code` via substitute()/eval() instead of


Merge this NOTE and the comments before?

schochastics and others added 5 commits June 25, 2026 15:12

Re-enable IGRAPH_CHECK_RNG_STATE env var guard for test_that RNG stat…

7d9cec9

…e checker

test: remove unused local_rng_version() helper

f1ca41f

Dead code: every local_rng_version("3.5.0") call was folded into igraph_local_seed(seed, rng_version = "3.5.0") earlier in this branch, leaving no call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

schochastics added the hackathon-topic 📆 label Jul 3, 2026

schochastics requested a review from maelle July 3, 2026 09:31

maelle reviewed Jul 3, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

schochastics commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

New helpers (tests/testthat/helper.R)

setup.R

Test files

tools/AGENTS.md

Notes

Uh oh!

maelle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

schochastics commented Jun 25, 2026 •

edited

Loading

New helpers (`tests/testthat/helper.R`)