Skip to content

test: add tests for walker errors - #2759

Merged
mightyiam merged 1 commit into
molybdenumsoftware:masterfrom
luuumine:errors
Sep 1, 2026
Merged

test: add tests for walker errors#2759
mightyiam merged 1 commit into
molybdenumsoftware:masterfrom
luuumine:errors

Conversation

@luuumine

Copy link
Copy Markdown

As discussed in #2756, this PR adds tests to document current program

  • when the path is wrong, it prints config error: path error: file not found: file.nix but exits successfully.
  • when a file cannot be read (e.g., due to missing permissions), it catches the I/O error, warns that the file contains non-utf8 content, and exits successfully at the end.

more regarding those issues:

  • in the first situation, making the program exit with an error code instead of success would simply be done by adding std::process::exit(1); in main.rs.
  • in the second situation, it currently skips unreadable files (but continues to lint the other files). This skipping behavior is good, so keeping the exit code as success makes sense. However, just reporting a read error on the file would be much better than throwing the current inaccurate utf-8 warning. the goal for this second issue would just be to change the inaccurate "utf-8 error" to a better "cannot read file" warning, but otherwise leaving it as is.

i believe the fix for those issues can wait since it is non that important and unrelated to #2756 (which do not change any of these behaviors)

@mightyiam mightyiam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, and thank you for the analysis. Do you think that the test should have comments that concisely explain how current behavior is undesirable?

@luuumine

luuumine commented Sep 1, 2026

Copy link
Copy Markdown
Author

I think it's fine to leave them as-is.
The behavior is fine enough for now ; and I will try to look at it in the next few weeks

@mightyiam
mightyiam merged commit 92d24df into molybdenumsoftware:master Sep 1, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants