Skip to content
Open
Changes from all commits
Commits
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
64 changes: 64 additions & 0 deletions process_report/tests/unit/invoices/test_pi_specific_invoice.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,67 @@ 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(
# First row has no PI and so should be skipped during export
pi=[None, "PI1"],
institution=["", "BU"],
balance=[0, 100],
)
Comment on lines +198 to +203

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")

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"

pi_inv = test_utils.new_pi_specific_invoice("test_dir", invoice_month)
pi_inv.export_s3(s3_bucket)
expected_path = "test_dir/BU_PI1_2025-01.pdf"
expected_s3_archive_path = (
"Invoices/2025-01/Archive/test_dir/BU_PI1_2025-01 2025-01-01.pdf"
)
expected_s3_path = "Invoices/2025-01/test_dir/BU_PI1_2025-01.pdf"
s3_bucket.upload_file.assert_any_call(expected_path, expected_s3_archive_path)
s3_bucket.upload_file.assert_any_call(expected_path, expected_s3_path)
assert s3_bucket.upload_file.call_count == 2

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