asl-node-auth-check: Tighten up key failures

[mkmer]

Add 100% coverage

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for missing or invalid node status data with detailed error messages.
  • Added conditional warnings when node registration information is unavailable.

• Tests

  • Added comprehensive test coverage for authentication check flows and utility functions.
  • Added tests for HTTP registration parsing and node ping verification.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ae17a859-bf08-401d-818e-e901c4d77442

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch node-auth-check

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py (2)

362-426: Consider using underscore prefix for unused unpacked variables.

Multiple tests unpack tuples but don't use all values. While this works, static analysis tools flag these. Using _ prefix for unused variables is a Python convention that silences these warnings and makes intent clearer.

Example fix:

-            host, perceived, state = _module.find_iax_registration("12345")
+            _host, _perceived, state = _module.find_iax_registration("12345")
             assert state == "Request Sent"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py` around lines
362 - 426, Several tests unpack the triple returned by
_module.find_iax_registration but only assert a subset of the values; update
those tests to use underscore-prefixed names for unused unpacked variables
(e.g., change "host, perceived, state = _module.find_iax_registration(...)" to
"_host, _perceived, state = _module.find_iax_registration(...)" or similar) in
the following test functions: test_iax_insufficient_lines,
test_iax_insufficient_columns, test_iax_subprocess_error (and any other tests in
this file that ignore some return values) so static analyzers stop flagging
unused variables while keeping assertions against the used names.

---

198-206: Remove extraneous f prefix from strings without placeholders.

Lines 198-206 contain f-strings that have no interpolation placeholders. These should be regular strings.

Proposed fix

-        main_content = f"""
+        main_content = """
 [12345] (node-main)

 `#include` subdir/sub.conf
 """
-        sub_content = """
+        sub_content = """
 [54321] (node-main)

 """

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py` around lines
198 - 206, The two string assignments main_content and sub_content use f-strings
but contain no interpolation; change their declarations from f"""...""" to
regular triple-quoted strings ("""...""") so main_content and sub_content are
plain strings without the unnecessary f-prefix.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py`:
- Around line 41-48: The run() helper creates buf = io.StringIO() but never
redirects stdout into it, so printed output from _module.entrypoint() is lost;
update run() to redirect sys.stdout to buf while calling _module.entrypoint()
(e.g., use contextlib.redirect_stdout(buf) or patch sys.stdout to buf inside the
with patch.dict(sys.modules) block) so that buf.getvalue() returns captured
output, preserving the existing SystemExit handling.
- Around line 79-90: The test test_main_valid_user is patching pwd.getpwnam but
the code calls require_root_or_asterisk (invoked by entrypoint) which uses
pwd.getpwuid(uid).pw_name, so update the test to patch pwd.getpwuid instead of
pwd.getpwnam; specifically, in test_main_valid_user replace the patch target
"pwd.getpwnam" with "pwd.getpwuid" and return a MagicMock with pw_uid, pw_gid,
and pw_name as before so require_root_or_asterisk sees the mocked username for
the given uid.

---

Nitpick comments:
In `@tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py`:
- Around line 362-426: Several tests unpack the triple returned by
_module.find_iax_registration but only assert a subset of the values; update
those tests to use underscore-prefixed names for unused unpacked variables
(e.g., change "host, perceived, state = _module.find_iax_registration(...)" to
"_host, _perceived, state = _module.find_iax_registration(...)" or similar) in
the following test functions: test_iax_insufficient_lines,
test_iax_insufficient_columns, test_iax_subprocess_error (and any other tests in
this file that ignore some return values) so static analyzers stop flagging
unused variables while keeping assertions against the used names.
- Around line 198-206: The two string assignments main_content and sub_content
use f-strings but contain no interpolation; change their declarations from
f"""...""" to regular triple-quoted strings ("""...""") so main_content and
sub_content are plain strings without the unnecessary f-prefix.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb3d48ca-82e1-41a6-94b1-28647c7b49d1

📥 Commits

Reviewing files that changed from the base of the PR and between 5368b3f and 5c4f284.

📒 Files selected for processing (4)

• bin/asl-node-auth-check
• tests/test_asl_node_auth_check/test_asl_node_auth_check_http.py
• tests/test_asl_node_auth_check/test_asl_node_auth_check_main.py
• tests/test_asl_node_auth_check/test_asl_node_auth_check_reachability.py