Skip to content

Merged duplicate create_external_session tests#18

Closed
chris-adam wants to merge 1 commit intomainfrom
PARAF-346/test_create_external_session
Closed

Merged duplicate create_external_session tests#18
chris-adam wants to merge 1 commit intomainfrom
PARAF-346/test_create_external_session

Conversation

@chris-adam
Copy link
Copy Markdown
Contributor

@chris-adam chris-adam commented Feb 26, 2026

Summary by CodeRabbit

  • Tests
    • Reworked external session creation tests to streamline scenarios and improve coverage.
    • Added and validated a no-files scenario to ensure correct handling when no session files exist.
    • Strengthened assertions for seal-related request URLs to verify outbound request targets.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d14c99a and ea863f7.

📒 Files selected for processing (1)
  • src/imio/esign/tests/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/imio/esign/tests/test_utils.py

📝 Walkthrough

Walkthrough

Test file refactor: removed a large multi-scenario test_create_external_session, added test_create_external_session_no_files for the no-files case, and added assertions validating the exact POST URL in test_create_external_session_seal_payload.

Changes

Cohort / File(s) Summary
Test Refactoring
src/imio/esign/tests/test_utils.py
Removed the comprehensive test_create_external_session suite; added test_create_external_session_no_files to assert _no_files_ is returned when all session files resolve to nothing; added assertions in test_create_external_session_seal_payload to verify the exact outbound POST URL (http://test.example.com/imio/esign/v1/luxtrust/sessions).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I hopped through tests with nimble paws,
Split a giant case into finer laws,
When files are gone, I softly call,
"no_files" answers, quiet and small,
I stamp the URL — tidy, neat, and all. 🐇

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Merged duplicate create_external_session tests' accurately describes the main change: consolidating and reorganizing redundant test cases for the create_external_session function.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PARAF-346/test_create_external_session

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/imio/esign/tests/test_utils.py (1)

569-577: Strengthen the no-files test by asserting no outbound POST is attempted.

This test validates the return value, but it should also lock in the early-return behavior by asserting requests.post is never called.

♻️ Suggested test hardening
 def test_create_external_session_no_files(self):
     """Returns _no_files_ when all file UIDs in the session resolve to nothing."""
     signers = [("user1", "user1@sign.com", "User 1", "Position 1")]
     sid, session = add_files_to_session(signers, (self.uids[0],))
     for i in range(len(session["files"])):
         session["files"][i]["uid"] = "nonexistent_uid_{}".format(i)
-    result = create_external_session(sid, esign_root_url="http://test.example.com")
+    with patch("imio.esign.utils.requests.post") as mock_post:
+        result = create_external_session(sid, esign_root_url="http://test.example.com")
+    mock_post.assert_not_called()
     self.assertEqual(result, "_no_files_")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/imio/esign/tests/test_utils.py` around lines 569 - 577, Update the
test_create_external_session_no_files to also assert that no outbound POST is
attempted: wrap the call to create_external_session with a mock/patch of the
requests.post used by create_external_session (patch the requests.post symbol in
the module that defines create_external_session, e.g., patch
'imio.esign.utils.requests.post'), call create_external_session(sid,
esign_root_url=...), assert the return is "_no_files_", and then assert the
mocked requests.post was never called; keep references to
test_create_external_session_no_files, create_external_session and
add_files_to_session to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/imio/esign/tests/test_utils.py`:
- Around line 569-577: Update the test_create_external_session_no_files to also
assert that no outbound POST is attempted: wrap the call to
create_external_session with a mock/patch of the requests.post used by
create_external_session (patch the requests.post symbol in the module that
defines create_external_session, e.g., patch 'imio.esign.utils.requests.post'),
call create_external_session(sid, esign_root_url=...), assert the return is
"_no_files_", and then assert the mocked requests.post was never called; keep
references to test_create_external_session_no_files, create_external_session and
add_files_to_session to locate the code.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2b70fe and d14c99a.

📒 Files selected for processing (1)
  • src/imio/esign/tests/test_utils.py

@chris-adam chris-adam force-pushed the PARAF-346/test_create_external_session branch from d14c99a to ea863f7 Compare February 26, 2026 14:24
@chris-adam chris-adam requested a review from sgeulette February 26, 2026 14:25
@chris-adam
Copy link
Copy Markdown
Contributor Author

Moved to test refactoring in #32

@chris-adam chris-adam closed this Mar 27, 2026
@chris-adam chris-adam deleted the PARAF-346/test_create_external_session branch March 27, 2026 12:41
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.

1 participant