Address review feedback: use helper for error snapshots and add test function by Copilot · Pull Request #2522 · igraph/rigraph

Copilot · 2026-01-22T10:10:53Z

Addresses review feedback on #2515 by consolidating error snapshot testing patterns and adding dedicated test infrastructure for error formatting with source location information.

Changes

Use existing test helper: Updated test-centrality.R to use expect_snapshot_igraph_error() instead of inline transform function
Add test function: Created test_error_with_source() that calls C code which throws an error with file:line information
- C function: Rx_igraph_test_error_with_source() in rinterface_extra.c
- R wrapper in utils.R (internal, not exported)
New test file: Added test-error-formatting.R with expect_snapshot(transform = ) to verify complete source location format (file + line number)

Example

# Error now includes source location
igraph:::test_error_with_source()
#> Error: Test error message for verifying source location formatting. Invalid value
#> Source: rinterface_extra.c:7766

Implementation Details

The test function is intentionally not exported - it's marked as internal and accessible via igraph:::test_error_with_source() for testing purposes only. The test uses a custom transform sub(":(\\d+)", ":<linenumber>", x) to preserve the file name while masking line numbers, ensuring we verify the complete source location format without line number brittleness.

By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Update test-centrality.R to use expect_snapshot_igraph_error() - Add test_error_with_source() function to demonstrate error formatting - Create test-error-formatting.R to test error formatting functionality - Update snapshot to use :xx format instead of :<linenumber> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

The error handler automatically adds punctuation via maybe_add_punctuation(), so we should not include it in the message itself. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

krlmlr

@copilot: Check if expect_snapshot_igraph_error() can be used in other locations instead of expect_snapshot(transform = ) .

krlmlr · 2026-01-24T09:19:46Z

NAMESPACE

 export(subgraph_isomorphic)
 export(subgraph_isomorphisms)
 export(tail_of)
+export(test_error_with_source)


This doesn't need to be exported.

Fixed in f628871. Removed the export - the function is now internal-only and accessible via igraph:::test_error_with_source() for testing.

krlmlr · 2026-01-24T09:20:58Z

tests/testthat/test-error-formatting.R

@@ -0,0 +1,5 @@
+test_that("error messages include source file and line information", {
+  expect_snapshot_igraph_error({


This needs to be a regular expect_snapshot(transform = ), the purpose of the test is to ensure that we don't lose information.

Fixed in f628871. Changed to use expect_snapshot(transform = function(x) sub(":(\\d+)", ":<linenumber>", x)) to preserve the file name while masking just the line number. The snapshot now shows Source: rinterface_extra.c:<linenumber> ensuring we verify the full source location format.

- Remove export of test_error_with_source() - not needed for internal test function - Change test-error-formatting.R to use expect_snapshot(transform=) instead of expect_snapshot_igraph_error() to preserve file name and verify line number format - Update snapshot to use <linenumber> format to ensure we don't lose information Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

Copilot · 2026-01-24T09:28:28Z

@copilot: Check if expect_snapshot_igraph_error() can be used in other locations instead of expect_snapshot(transform = ) .

Checked in f628871. Searched for all uses of expect_snapshot.*transform.*= in test files and found no other locations that should use expect_snapshot_igraph_error(). The helper is currently used only in test-centrality.R as intended.

Copilot

Pull request overview

This PR addresses review feedback from #2515 by consolidating error snapshot testing patterns and adding test infrastructure for verifying error formatting with source location information (file:line).

Changes:

Refactored test-centrality.R to use the existing expect_snapshot_igraph_error() helper instead of an inline transform
Added test_error_with_source() test function with C implementation that throws an error with source location
Created new test file test-error-formatting.R to verify complete source location format

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/testthat/test-error-formatting.R	New test file to verify error messages include source file and line information
tests/testthat/_snaps/error-formatting.md	Snapshot file for error formatting test
tests/testthat/test-centrality.R	Refactored to use `expect_snapshot_igraph_error()` helper
tests/testthat/_snaps/centrality.md	Updated snapshot to use `:xx` pattern consistently
src/rinterface_extra.c	Added C test function `Rx_igraph_test_error_with_source()`
src/cpp11.cpp	Registered new test function in R's call table
R/utils.R	Added R wrapper function for test (internal, not exported)

Copilot · 2026-01-24T09:41:51Z

tests/testthat/test-error-formatting.R

@@ -0,0 +1,8 @@
+test_that("error messages include source file and line information", {
+  expect_snapshot(
+    error = TRUE,


The test is missing a transform parameter to replace line numbers in the snapshot. The snapshot file shows <linenumber>, but without a transform, the snapshot will contain actual line numbers that will change when the C code is modified.

According to the PR description, this test should use transform = function(x) sub(":(\\d+)", ":<linenumber>", x), but this is missing from the test. Alternatively, consider using the existing helper function expect_snapshot_igraph_error() which already handles this transformation (though it uses :xx instead of :<linenumber>).

Suggested change

error = TRUE,

error = TRUE,

transform = function(x) sub(":(\\d+)", ":<linenumber>", x),

Copilot · 2026-01-24T09:41:51Z

tests/testthat/_snaps/error-formatting.md

+    Condition
+      Error in `test_error_with_source()`:
+      ! Test error message for verifying source location formatting. Invalid value
+      Source: rinterface_extra.c:<linenumber>


The snapshot uses <linenumber> as a placeholder, but the codebase convention is to use :xx. The helper function expect_snapshot_igraph_error() (tests/testthat/helper.R:47-56) uses gsub(":(\\d+)", ":xx", y), and this pattern is used in the vast majority of error snapshots (e.g., tests/testthat/_snaps/aaa-auto.md and tests/testthat/_snaps/centrality.md).

Only a couple of tests in test-games.R use <linenumber>, which appears to be an exception to the established convention. For consistency, consider using the expect_snapshot_igraph_error() helper and updating the snapshot to use :xx instead of <linenumber>.

Initial plan

afab014

Copilot AI assigned Copilot and krlmlr Jan 22, 2026

Copilot AI mentioned this pull request Jan 22, 2026

feat: improve formatting of error messages from C #2515

Merged

Copilot started work on behalf of krlmlr January 22, 2026 10:11 View session

Copilot AI and others added 2 commits January 22, 2026 10:30

fix: remove punctuation from test error message

331d876

The error handler automatically adds punctuation via maybe_add_punctuation(), so we should not include it in the message itself. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Address review feedback and add tests for error formatting functionality~~ Address review feedback: use helper for error snapshots and add test function Jan 22, 2026

Copilot AI requested a review from krlmlr January 22, 2026 10:37

Copilot finished work on behalf of krlmlr January 22, 2026 10:37

krlmlr requested changes Jan 24, 2026

View reviewed changes

Copilot started work on behalf of krlmlr January 24, 2026 09:22 View session

Copilot AI requested a review from krlmlr January 24, 2026 09:30

Copilot finished work on behalf of krlmlr January 24, 2026 09:30

Tweaks

98a83e4

krlmlr marked this pull request as ready for review January 24, 2026 09:37

Copilot AI review requested due to automatic review settings January 24, 2026 09:37

krlmlr merged commit 4068aee into better Jan 24, 2026
1 check passed

krlmlr deleted the copilot/sub-pr-2515 branch January 24, 2026 09:37

Copilot started reviewing on behalf of krlmlr January 24, 2026 09:37 View session

Copilot AI reviewed Jan 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Address review feedback: use helper for error snapshots and add test function#2522

Address review feedback: use helper for error snapshots and add test function#2522
krlmlr merged 5 commits intobetterfrom
copilot/sub-pr-2515

Copilot AI commented Jan 22, 2026 •

edited

Loading

Uh oh!

krlmlr left a comment

Uh oh!

krlmlr Jan 24, 2026

Uh oh!

Copilot AI Jan 24, 2026

Uh oh!

krlmlr Jan 24, 2026

Uh oh!

Copilot AI Jan 24, 2026

Uh oh!

Copilot AI commented Jan 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 24, 2026

Uh oh!

Copilot AI Jan 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,5 @@
		test_that("error messages include source file and line information", {
		expect_snapshot_igraph_error({

	error = TRUE,
	error = TRUE,
	transform = function(x) sub(":(\\d+)", ":<linenumber>", x),

Uh oh!

Conversation

Copilot AI commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Example

Implementation Details

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 22, 2026 •

edited

Loading