Refactoring verification module by codedbyishika · Pull Request #60 · Imageomics/cautious-robot

codedbyishika · 2026-06-05T20:44:26Z

Refactor download verification module: move remaining post-download verification functions from __main__.py to buddy_check.py
Change imports
Edit AGENTS.md
Include unit tests

Copilot

Pull request overview

This PR refactors the post-download verification workflow by moving checksum generation and download verification helpers out of __main__.py into buddy_check.py, updating references/documentation accordingly, and adding unit tests to cover the moved functionality.

Changes:

Moved process_checksums() and verify_downloads() from __main__.py to buddy_check.py.
Updated tests to patch get_checksums / BuddyCheck in the new module location and added a new test suite for verification helpers.
Updated AGENTS.md to reflect the new module responsibilities and documented the new helper functions.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tests/test_verify_downloads.py`	Adds unit tests for `process_checksums()` and `verify_downloads()` after refactor.
`tests/test_download_images.py`	Updates patch targets to reflect moved checksum/verification code.
`src/cautiousrobot/buddy_check.py`	Adds `process_checksums()` and `verify_downloads()` and imports `get_checksums`.
`src/cautiousrobot/__main__.py`	Removes inlined checksum/verification helpers; imports them from `buddy_check.py`.
`AGENTS.md`	Updates architecture docs to reflect refactor and new helper functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This reverts commit 1a75a3a.

egrace479

Some notes on the test case setups.

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

- change "md5" to "hash" - specify error printouts - remove unused test

egrace479

We should probably add a test that calls verify_downloads with filename_col != "filename" to ensure something like this is caught in the future.

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

Copilot

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

thompsonmj · 2026-07-02T18:34:35Z

+        self.assertEqual(call_kwargs["source_validation_col"], "hash")
+
+    @patch('cautiousrobot.buddy_check.BuddyCheck')
+    def test_verify_downloads_calls_buddy_check_with_correct_params(self, mock_buddy_check_class):


Seems redundant to test_verify_downloads_uses_correct_algorithm() and test_verify_downloads_with_custom_filename_col(). Would recommend removing test_verify_downloads_calls_buddy_check_with_correct_params().

Removed redundant tests

Retrieved test_verify_downloads_uses_correct_algorithm and test_verify_downloads_with_custom_filename_col tests and removed test_verify_downloads_calls_buddy_check_with_correct_params

thompsonmj · 2026-07-07T15:18:50Z

+        )
+
+    @patch('cautiousrobot.buddy_check.get_checksums')
+    def test_process_checksums_different_algorithm(self, mock_get_checksums):


I think this would work better as an integration test on the CLI -a input (actually execute a checksum) rather than a mock. Here, it does not really test a different checksum algorithm. Going through a real #sum-buddy or BuddyCheck could actually break on non-md5, which would make a more valuable test.

For this PR, I would suggest removing this unit test, then we can revisit the checksum algorithm contract that passes through the code and make corresponding integration tests there.

Removed test temporarily and will be addressed in the new issue

egrace479 · 2026-07-07T15:31:18Z

+        shutil.rmtree(self.temp_dir)
+
+    @patch('cautiousrobot.buddy_check.get_checksums')
+    def test_process_checksums_mismatched_counts(self, mock_get_checksums):


This test should be added to the TestProcessChecksums class.

Moved the test to TestProcessChecksums

- Update docstrings from main branch - Update tests according to comments

codedbyishika added 3 commits June 5, 2026 16:40

Refactor verify_downloads.py

ff9ce09

Update test calls

fb2e654

Remove unused variables

f74d66c

codedbyishika self-assigned this Jun 5, 2026

Include variable name

1186e25

codedbyishika requested a review from egrace479 June 5, 2026 20:51

Move download verification functions into buddy_check.py

08c9f03

codedbyishika linked an issue Jun 8, 2026 that may be closed by this pull request

Move download verification functions into buddy_check.py #62

Open

egrace479 requested a review from Copilot June 9, 2026 17:13

Copilot started reviewing on behalf of egrace479 June 9, 2026 17:13 View session

Copilot AI reviewed Jun 9, 2026

View reviewed changes

Comment thread src/cautiousrobot/buddy_check.py Outdated

Comment thread src/cautiousrobot/buddy_check.py Outdated

Comment thread tests/test_verify_downloads.py Outdated

egrace479 reviewed Jun 9, 2026

View reviewed changes

Comment thread src/cautiousrobot/buddy_check.py Outdated

egrace479 reviewed Jun 9, 2026

View reviewed changes

Comment thread src/cautiousrobot/buddy_check.py Outdated

Revise error handling descriptions

9f43470

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

egrace479 reviewed Jun 9, 2026

View reviewed changes

Comment thread src/cautiousrobot/buddy_check.py Outdated

egrace479 and others added 2 commits June 9, 2026 14:33

Clarify return on error

9c9a222

Potential fix for pull request finding

1a75a3a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

egrace479 reviewed Jun 9, 2026

View reviewed changes

Comment thread tests/test_verify_downloads.py

Revert "Potential fix for pull request finding"

5ceba99

This reverts commit 1a75a3a.

egrace479 reviewed Jun 10, 2026

View reviewed changes

Comment thread tests/test_verify_downloads.py Outdated

Comment thread tests/test_verify_downloads.py

Comment thread tests/test_verify_downloads.py Outdated

Comment thread tests/test_verify_downloads.py

Comment thread tests/test_verify_downloads.py Outdated

codedbyishika and others added 3 commits June 10, 2026 17:18

Update tests/test_verify_downloads.py

7b46454

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

Update tests/test_verify_downloads.py

8510b5a

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

Updates for tests

0f488a0

- change "md5" to "hash" - specify error printouts - remove unused test

egrace479 requested changes Jun 10, 2026

View reviewed changes

Comment thread src/cautiousrobot/buddy_check.py Outdated

codedbyishika and others added 3 commits June 11, 2026 13:11

Update src/cautiousrobot/buddy_check.py

16715be

Co-authored-by: Elizabeth Campolongo <38985481+egrace479@users.noreply.github.com>

Add new test to check filename

fcb7e4f

Update tests according to the change in buddy_id

b49d493

codedbyishika requested a review from egrace479 June 11, 2026 17:22

egrace479 requested a review from Copilot June 11, 2026 19:27

Copilot started reviewing on behalf of egrace479 June 11, 2026 19:28 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread tests/test_verify_downloads.py

Comment thread tests/test_verify_downloads.py Outdated

codedbyishika and others added 2 commits June 11, 2026 17:19

Potential fix for pull request finding

1551f1e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

23b7bf0

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

egrace479 requested a review from thompsonmj June 16, 2026 15:11

thompsonmj requested changes Jul 2, 2026

View reviewed changes

codedbyishika added 2 commits July 3, 2026 13:15

Remove redundant functions

cac6e3d

Include docstrings and comments

e5b076d

thompsonmj requested changes Jul 7, 2026

View reviewed changes

thompsonmj mentioned this pull request Jul 7, 2026

Handling and testing other algorithms #68

Open

egrace479 reviewed Jul 7, 2026

View reviewed changes

Update tests and docstrings

7696ffa

- Update docstrings from main branch - Update tests according to comments

Uh oh!

Conversation

codedbyishika commented Jun 5, 2026 • edited by egrace479 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egrace479 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egrace479 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

thompsonmj Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

codedbyishika Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

codedbyishika Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

thompsonmj Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

codedbyishika Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egrace479 Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

codedbyishika Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codedbyishika commented Jun 5, 2026 •

edited by egrace479

Loading

codedbyishika Jul 7, 2026 •

edited

Loading