Skip to content

fix: scrub file paths in error/warning snapshots and fix spacing #2538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
krlmlr merged 5 commits into from
Jan 24, 2026
Merged

fix: scrub file paths in error/warning snapshots and fix spacing #2538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca9ff03
fix: scrub file paths in error/warning snapshots and fix spacing
Copilot Jan 24, 2026
71d9fcf
refactor: extract scrub_igraph_file_paths helper to reduce duplication
Copilot Jan 24, 2026
7937564
docs: clarify optional space in regex pattern comment
Copilot Jan 24, 2026
280ce43
docs: add FIXME comments for HRG error tests with assertion failures
Copilot Jan 24, 2026
642b44b
Merge branch 'main' into copilot/sub-pr-2515
krlmlr Jan 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tests/testthat/_snaps/aaa-auto.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5804,7 +5804,7 @@
hrg_sample_impl(hrg = NULL)
Condition
Error in `hrg_sample_impl()`:
! At <file>:<line> : Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
! At <file>:<line>: Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Copy link
Copy Markdown
Contributor

@krlmlr krlmlr Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the assertion here and below fail? If it's a bug, add a FIXME to the test. If this can never occur in practice, change the test.

Copy link
Copy Markdown
Contributor Author

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertions fail because passing NULL to these HRG functions triggers an assertion failure in the C code (the R wrapper converts NULL to an empty HRG structure, which the C code doesn't handle properly). This is a bug in the C code - it should validate the input and return a proper error message instead of an assertion failure. Added FIXME comments in 280ce43 to document this issue.

Please restart your R session to avoid crashes or other surprising behavior.

# hrg_sample_many_impl basic
Expand Down Expand Up @@ -5835,7 +5835,7 @@
hrg_sample_many_impl(hrg = NULL, num_samples = 2)
Condition
Error in `hrg_sample_many_impl()`:
! At <file>:<line> : Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
! At <file>:<line>: Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Please restart your R session to avoid crashes or other surprising behavior.

# hrg_game_impl basic
Expand All @@ -5857,7 +5857,7 @@
hrg_game_impl(hrg = NULL)
Condition
Error in `hrg_game_impl()`:
! At <file>:<line> : Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
! At <file>:<line>: Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Please restart your R session to avoid crashes or other surprising behavior.

# hrg_consensus_impl errors
Expand Down Expand Up @@ -11198,7 +11198,7 @@
Condition
Warning in `connect_neighborhood_impl()`:
Order smaller than two, graph will be unchanged.
Source: operators/connect_neighborhood.c:85
Source: <file>:<line>
Output
IGRAPH U--- 5 5 -- Ring graph
+ attr: name (g/c), mutual (g/l), circular (g/l)
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/games.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Condition
Error in `sample_degseq()`:
! Cannot make a connected graph from the given degree sequence. Invalid value
Source: games/degree_sequence_vl/gengraph_mr-connected.cpp:<linenumber>
Source: <file>:<line>

# sample_degseq() works -- Power-law degree error

Expand All @@ -14,5 +14,5 @@
Condition
Error in `sample_degseq()`:
! Cannot realize the given degree sequence as an undirected, simple graph. Invalid value
Source: games/degree_sequence_vl/gengraph_mr-connected.cpp:<linenumber>
Source: <file>:<line>

29 changes: 18 additions & 11 deletions tests/testthat/helper.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,28 @@ expect_not_identical_graphs <- function(g1, g2, ...) {
expect_false(identical_graphs(g1, g2, ...))
}

scrub_igraph_file_paths <- function(y) {
# Scrub file name and line number from error/warning messages
# Handles "Source: filename:linenumber" and "At path/to/file:line :" patterns
# The "At" pattern may have an optional space before the final colon
y <- gsub("Source: [^:]+:(\\d+|xx|<linenumber>)", "Source: <file>:<line>", y)
y <- gsub("At [^:]+:(\\d+|xx) ?:", "At <file>:<line>:", y)
Comment on lines +51 to +52
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scrub_igraph_file_paths() uses [^:]+ to match the file path, which will fail to scrub paths that contain a colon (e.g., Windows drive paths like C:/.../file.c:123). This can leave unsanitized paths/line numbers in snapshots and potentially make tests OS-dependent. Consider matching up to the last :<line> segment instead (e.g., using a greedy match before the line number, or otherwise allowing colons in the path).

Suggested change
y <- gsub("Source: [^:]+:(\\d+|xx|<linenumber>)", "Source: <file>:<line>", y)
y <- gsub("At [^:]+:(\\d+|xx) ?:", "At <file>:<line>:", y)
y <- gsub("Source: .+:(\\d+|xx|<linenumber>)", "Source: <file>:<line>", y)
y <- gsub("At .+:(\\d+|xx) ?:", "At <file>:<line>:", y)

Copilot uses AI. Check for mistakes.
y
}

expect_snapshot_igraph_error <- function(x, ...) {
inject(expect_snapshot(
{{ x }},
error = TRUE,
transform = function(y) {
# Scrub file name and line number from error/warning messages
# Handles "Source: filename:linenumber" and "At path/to/file:line :" patterns
y <- gsub(
"Source: [^:]+:(\\d+|xx|<linenumber>)",
"Source: <file>:<line>",
y
)
y <- gsub("At [^:]+:(\\d+|xx) :", "At <file>:<line> :", y)
y
},
transform = scrub_igraph_file_paths,
...
))
}

expect_snapshot_igraph <- function(x, ...) {
inject(expect_snapshot(
{{ x }},
transform = scrub_igraph_file_paths,
...
))
}
11 changes: 10 additions & 1 deletion tests/testthat/test-aaa-auto.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6733,6 +6733,9 @@ test_that("hrg_sample_impl errors", {
skip_if(Sys.getenv("R_SANITIZER") == "true")
withr::local_seed(20250909)
local_igraph_options(print.id = FALSE)
# FIXME: This test triggers an assertion failure in the C code when passing
# NULL/empty HRG. The C code should validate input and return a proper error
# message instead of an assertion failure.
expect_snapshot_igraph_error(hrg_sample_impl(
hrg = NULL
))
Expand Down Expand Up @@ -6763,6 +6766,9 @@ test_that("hrg_sample_many_impl errors", {
skip_if(Sys.getenv("R_SANITIZER") == "true")
withr::local_seed(20250909)
local_igraph_options(print.id = FALSE)
# FIXME: This test triggers an assertion failure in the C code when passing
# NULL/empty HRG. The C code should validate input and return a proper error
# message instead of an assertion failure.
expect_snapshot_igraph_error(hrg_sample_many_impl(
hrg = NULL,
num_samples = 2
Expand Down Expand Up @@ -6792,6 +6798,9 @@ test_that("hrg_game_impl errors", {
skip_if(Sys.getenv("R_SANITIZER") == "true")
withr::local_seed(20250909)
local_igraph_options(print.id = FALSE)
# FIXME: This test triggers an assertion failure in the C code when passing
# NULL/empty HRG. The C code should validate input and return a proper error
# message instead of an assertion failure.
expect_snapshot_igraph_error(hrg_game_impl(
hrg = NULL
))
Expand Down Expand Up @@ -11044,7 +11053,7 @@ test_that("connect_neighborhood_impl basic", {
withr::local_seed(20250909)
local_igraph_options(print.id = FALSE)
g <- make_ring(5)
expect_snapshot(connect_neighborhood_impl(
expect_snapshot_igraph(connect_neighborhood_impl(
graph = g,
order = 1,
mode = c("all", "out", "in")
Expand Down
20 changes: 6 additions & 14 deletions tests/testthat/test-games.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,9 @@ test_that("sample_degseq() works -- exponential degree error", {
replace = TRUE,
prob = exp(-0.5 * (1:100))
)
expect_snapshot(
{
sample_degseq(exponential_degrees, method = "vl")
},
error = TRUE,
transform = function(x) sub("\\:[0-9]+", ":<linenumber>", x)
)
expect_snapshot_igraph_error({
sample_degseq(exponential_degrees, method = "vl")
})
})

test_that("sample_degseq() works -- Power-law degree ok", {
Expand All @@ -91,13 +87,9 @@ test_that("sample_degseq() works -- Power-law degree error", {
withr::local_seed(7)
powerlaw_degrees <- sample(1:100, 100, replace = TRUE, prob = (1:100)^-2)

expect_snapshot(
{
sample_degseq(powerlaw_degrees, method = "vl")
},
error = TRUE,
transform = function(x) sub("\\:[0-9]+", ":<linenumber>", x)
)
expect_snapshot_igraph_error({
sample_degseq(powerlaw_degrees, method = "vl")
})
})

test_that("sample_degseq() works -- fast.heur.simple", {
Expand Down
Loading