refactor: scrub file names in expect_snapshot_igraph_error()#2537
refactor: scrub file names in expect_snapshot_igraph_error()#2537
Conversation
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the expect_snapshot_igraph_error() test helper function to scrub file paths and line numbers from error messages in test snapshots, making them independent of build directory structures and more stable across different environments.
Changes:
- Updated the transform function in
expect_snapshot_igraph_error()to replace file paths and line numbers with placeholders<file>and<line> - Regenerated 52 error message snapshots across test files to use the new sanitized format
- Added
tests/testthat/_problems/to.gitignoreto exclude testthat's problem report directory
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/testthat/helper.R | Enhanced transform function with two regex patterns to scrub "Source:" and "At" file path patterns, with backward compatibility for legacy placeholders |
| tests/testthat/_snaps/centrality.md | Updated 1 error snapshot to use <file>:<line> placeholders |
| tests/testthat/_snaps/aaa-auto.md | Updated 51 error snapshots to use <file>:<line> placeholders |
| .gitignore | Added testthat problem reports directory to exclusions |
| # 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) |
There was a problem hiding this comment.
The regex pattern for the "At" pattern on line 55 only includes xx as a legacy placeholder but not <linenumber>, while the "Source" pattern on line 54 includes both xx and <linenumber>. For consistency and to handle all cases mentioned in the PR description, consider updating the pattern to: "At [^:]+:(\\d+|xx|<linenumber>) :". This ensures both patterns handle the same set of legacy placeholders.
| y <- gsub("At [^:]+:(\\d+|xx) :", "At <file>:<line> :", y) | |
| y <- gsub("At [^:]+:(\\d+|xx|<linenumber>) :", "At <file>:<line> :", y) |
Enhanced
expect_snapshot_igraph_error()to scrub file paths from error messages in test snapshots, making them independent of build directory structure and more stable across environments.Changes
Source: filename:linenumber→Source: <file>:<line>At path/to/file:line :→At <file>:<line> :aaa-auto.mdandcentrality.md) to use the new formatThe regex handles legacy placeholders (
xx,<linenumber>) for compatibility with tests using custom transforms.Example
Before:
After:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.