refactor: standardize error snapshot testing with expect_snapshot_igraph_error() (#2528)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

Author: Copilot

src/rinterface_extra.c

@@ -7760,13 +7760,6 @@ SEXP Rx_igraph_create_bipartite(SEXP types, SEXP edges, SEXP directed) {
   return R_igraph_create_bipartite(types, edges, directed);
 }
 
-/* Test function to verify error formatting with file and line information */
-attribute_visible SEXP Rx_igraph_test_error_with_source(void) {
-  igraph_errorf("Test error message for verifying source location formatting",
-                __FILE__, __LINE__, IGRAPH_EINVAL);
-  return R_NilValue;
-}
-
 SEXP Rx_igraph_finalizer(void) {
   return R_igraph_finalizer();
 }

src/sources-glue-c.mk

@@ -1 +1 @@
-GLUE_C_SOURCES=rcallback.o rinterface.o rinterface_extra.o rrandom.o uuid.o
+GLUE_C_SOURCES=rcallback.o rinterface.o rinterface_extra.o rrandom.o test_error_with_source.o uuid.o

src/test_error_with_source.c

@@ -0,0 +1,33 @@
+/* -*- mode: C -*-  */
+/*
+   IGraph library R interface.
+   Copyright (C) 2013  Gabor Csardi <csardi.gabor@gmail.com>
+   334 Harvard street, Cambridge, MA 02139 USA
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+   02110-1301 USA
+
+*/
+
+#include "rinterface.h"
+
+#include <R_ext/Visibility.h>
+
+/* Test function to verify error formatting with file and line information */
+attribute_visible SEXP Rx_igraph_test_error_with_source(void) {
+  igraph_errorf("Test error message for verifying source location formatting",
+                __FILE__, __LINE__, IGRAPH_EINVAL);
+  return R_NilValue;
+}

src/test_error_with_source.dd

@@ -0,0 +1,4 @@
+# Generated by deps.mk, do not edit by hand and do not add dependencies to system headers
+test_error_with_source.o: \
+  rinterface.h \
+  test_error_with_source.c \

tests/testthat/_snaps/error-formatting.md

@@ -5,5 +5,5 @@
     Condition
       Error in `test_error_with_source()`:
       ! Test error message for verifying source location formatting. Invalid value
-      Source: rinterface_extra.c:7766
+      Source: test_error_with_source.c:31
 

tests/testthat/test-adjacency.R

@@ -787,7 +787,7 @@ test_that("sparse/dense matrices min/max/plus", {
 
 test_that("graph_from_adjacency_matrix errors for NAs", {
   A <- matrix(c(1, 1, NA, 1), 2, 2)
-  expect_snapshot(graph_from_adjacency_matrix(A), error = TRUE)
+  expect_snapshot_igraph_error(graph_from_adjacency_matrix(A))
 })
 
 test_that("graph_from_adjacency_matrix handles add.colnames and add.rownames = FALSE correctly", {

tests/testthat/test-attributes.R

@@ -436,13 +436,13 @@ test_that("adding and removing attributes", {
 
 test_that("error messages work", {
   g <- make_full_graph(5)
-  expect_snapshot(set_vertex_attr(g, "test", value = c(1, 2)), error = TRUE)
-  expect_snapshot(set_edge_attr(g, "test", value = c(1, 2)), error = TRUE)
-  expect_snapshot(delete_graph_attr(g, "a"), error = TRUE)
-  expect_snapshot(delete_vertex_attr(g, "a"), error = TRUE)
-  expect_snapshot(delete_edge_attr(g, "a"), error = TRUE)
-  expect_snapshot(assert_named_list("a"), error = TRUE)
-  expect_snapshot(assert_named_list(list("a", "b")), error = TRUE)
+  expect_snapshot_igraph_error(set_vertex_attr(g, "test", value = c(1, 2)))
+  expect_snapshot_igraph_error(set_edge_attr(g, "test", value = c(1, 2)))
+  expect_snapshot_igraph_error(delete_graph_attr(g, "a"))
+  expect_snapshot_igraph_error(delete_vertex_attr(g, "a"))
+  expect_snapshot_igraph_error(delete_edge_attr(g, "a"))
+  expect_snapshot_igraph_error(assert_named_list("a"))
+  expect_snapshot_igraph_error(assert_named_list(list("a", "b")))
 })
 
 test_that("empty returns work", {
@@ -465,7 +465,7 @@ test_that("assign data.frame attributes works", {
 
 test_that("good error message when not using character", {
   ring <- graph_from_literal(A - B - C - D - E - F - G - A)
-  expect_snapshot(error = TRUE, {
+  expect_snapshot_igraph_error({
     set_graph_attr(ring, 1, 1)
   })
 })
@@ -476,7 +476,7 @@ test_that("set_vertex_attrs() works", {
   expect_equal(V(g)$color, rep("blue", vcount(g)))
   expect_equal(V(g)$size, rep(10, vcount(g)))
   expect_equal(V(g)$name, LETTERS[1:10])
-  expect_snapshot(error = TRUE, {
+  expect_snapshot_igraph_error({
     set_vertex_attrs(g)
   })
 

tests/testthat/test-centrality.R

@@ -848,7 +848,7 @@ test_that("arpack() errors well", {
   expect_snapshot_igraph_error({
     arpack(f, options = list(nev = 2, ncv = 4), sym = TRUE)
   })
-  expect_snapshot(error = TRUE, {
+  expect_snapshot_igraph_error({
     arpack(
       f,
       options = list(unknown_thing1 = 2, unknown_thing2 = 4),

tests/testthat/test-conversion.R

@@ -142,10 +142,10 @@ test_that("as_adjacency_matrix() works -- sparse + not both", {
 
 test_that("as_adjacency_matrix() errors well -- sparse", {
   g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE)
-  expect_snapshot(as_adjacency_matrix(g, attr = "bla"), error = TRUE)
+  expect_snapshot_igraph_error(as_adjacency_matrix(g, attr = "bla"))
 
   E(g)$bla <- letters[1:ecount(g)]
-  expect_snapshot(as_adjacency_matrix(g, attr = "bla"), error = TRUE)
+  expect_snapshot_igraph_error(as_adjacency_matrix(g, attr = "bla"))
 })
 
 test_that("as_adjacency_matrix() works -- sparse undirected", {
@@ -197,15 +197,13 @@ test_that("as_adjacency_matrix() works -- dense", {
 
 test_that("as_adjacency_matrix() errors well -- dense", {
   g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE)
-  expect_snapshot(
-    as_adjacency_matrix(g, attr = "bla", sparse = FALSE),
-    error = TRUE
+  expect_snapshot_igraph_error(
+    as_adjacency_matrix(g, attr = "bla", sparse = FALSE)
   )
 
   E(g)$bla <- letters[1:ecount(g)]
-  expect_snapshot(
-    as_adjacency_matrix(g, attr = "bla", sparse = FALSE),
-    error = TRUE
+  expect_snapshot_igraph_error(
+    as_adjacency_matrix(g, attr = "bla", sparse = FALSE)
   )
 })
 
@@ -679,7 +677,7 @@ test_that("edge names work", {
 
 test_that("graph_from_edgelist errors for NAs", {
   A <- matrix(c(1, 2, NA, 1), 2, 2)
-  expect_snapshot(graph_from_edgelist(A), error = TRUE)
+  expect_snapshot_igraph_error(graph_from_edgelist(A))
 })
 
 test_that("graph_from_data_frame works with factors", {

tests/testthat/test-error-formatting.R

@@ -1,4 +1,8 @@
 test_that("error messages include source file and line information", {
+  # Note: We use expect_snapshot(error = TRUE) here instead of
+  # expect_snapshot_igraph_error() because we need to test the actual
+  # line numbers in the error message, which expect_snapshot_igraph_error()
+  # would transform to "xx".
   expect_snapshot(
     error = TRUE,
     {