Skip to content

Add unit tests for PISpecificInvoice #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcoagonzales007 wants to merge 2 commits into
base: main
Choose a base branch
from
+64 −0
Open

Add unit tests for PISpecificInvoice #305

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f88c695
Add unit tests for PISpecificInvoice
marcoagonzales007 Jun 11, 2026
db2ce69
Address PR feedback for PISpecificInvoice tests
marcoagonzales007 Jun 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions process_report/tests/unit/invoices/test_pi_specific_invoice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,65 @@ def test_process_no_warnings(
):
pi_inv.process()
pi_inv.export()

@mock.patch("process_report.invoices.invoice.Invoice._filter_columns")
@mock.patch("os.path.exists")
@mock.patch("subprocess.run")
def test_export_missing_pi(
self, mock_subprocess_run, mock_path_exists, mock_filter_cols
):
invoice_month = "2025-01"
test_invoice = self._get_test_invoice(
pi=[None, "PI1"],
institution=["", "BU"],
balance=[0, 100],
)
Comment on lines +198 to +203

@QuanMPhm QuanMPhm Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a comment indicating that the test data is written such that one row has a missing PI? When I read it, it was not immediately clear what the test case was doing

mock_path_exists.return_value = True
mock_filter_cols.return_value = test_invoice
with tempfile.TemporaryDirectory() as test_dir:
pi_inv = test_utils.new_pi_specific_invoice(
test_dir, invoice_month, data=test_invoice
)
pi_inv.process()
pi_inv.export()
assert mock_subprocess_run.call_count == 1

@mock.patch("process_report.invoices.invoice.Invoice._filter_columns")
@mock.patch("os.path.exists")
@mock.patch("subprocess.run")

@QuanMPhm QuanMPhm Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

subprocess.run does not need to be mocked in this test

def test_export_no_chrome(
self, mock_subprocess_run, mock_path_exists, mock_filter_cols
):
invoice_month = "2025-01"
test_invoice = self._get_test_invoice(
pi=[None, "PI1"],
institution=["", "BU"],
balance=[0, 100],
)
mock_path_exists.return_value = False
mock_filter_cols.return_value = test_invoice
with tempfile.TemporaryDirectory() as test_dir:
pi_inv = test_utils.new_pi_specific_invoice(
test_dir, invoice_month, data=test_invoice
)
pi_inv.process()
with self.assertRaises(SystemExit):
pi_inv.export()

@mock.patch("process_report.util.get_iso8601_time")
@mock.patch("os.listdir")
def test_export_s3(self, mock_listdir, mock_get_time):
mock_get_time.return_value = "2025-01-01"
mock_listdir.return_value = ["BU_PI1_2025-01.pdf"]
s3_bucket = mock.MagicMock()
invoice_month = "2025-01"
test_invoice = self._get_test_invoice(
pi=["PI1"],
institution=["BU"],
balance=[100],
)

@QuanMPhm QuanMPhm Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given how PIInvoice is written, it is not necessary to create a test invoice for this test

pi_inv = test_utils.new_pi_specific_invoice(
"test_dir", invoice_month, data=test_invoice
)
pi_inv.export_s3(s3_bucket)
assert s3_bucket.upload_file.call_count == 2

@QuanMPhm QuanMPhm Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you make an assertion on the arguments that s3_bucket.upload_file() was called with? Otherwise, the mocking of process_report.util.get_iso8601_time was not necessary here.

Loading