Skip to content

Add unit tests for MOCAPrepaidInvoice#297

Merged
QuanMPhm merged 1 commit into
CCI-MOC:mainfrom
marcoagonzales007:add-test-mocaprepaidinvoice
Jun 15, 2026
Merged

Add unit tests for MOCAPrepaidInvoice#297
QuanMPhm merged 1 commit into
CCI-MOC:mainfrom
marcoagonzales007:add-test-mocaprepaidinvoice

Conversation

@marcoagonzales007

Copy link
Copy Markdown
Contributor

part of #185

Adds unit tests for MOCAPrepaidInvoice, bringing coverage from 73% to 100%.

Tests Added:

  1. test_prepare_export : verifies that only rows where MGHPCC Managed is False are included in the export
  2. test_output_path : verifies the output filename is correctly formatted with the invoice month
  3. test_output_s3_key : verifies the S3 path is correctly formatted
  4. test_output_s3_archive_key : verifies the S3 archive path is correctly formatted

Also adds new_moca_prepaid_invoice() to tests/util.py to follow the existing pattern used by all other invoices.

@QuanMPhm QuanMPhm left a comment

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.

Looks good! I would suggest that you include your PR description into the commit message. When people look back at commit histories, the commit message is usually more easily accessible than a PR description.

@QuanMPhm QuanMPhm requested review from knikolla and naved001 June 4, 2026 13:36

@knikolla knikolla left a comment

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.

Great work!


def new_moca_prepaid_invoice(
name="",
invoice_month="0000-00",

@knikolla knikolla Jun 9, 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.

Nitpick: This invoice month default parameter is highly unrealistic as no month 0 can exist. When implementing test data like this it is usually a good idea to stick with data that would be acceptable during normal operation.

@QuanMPhm

Copy link
Copy Markdown
Contributor

@marcoagonzales007 Can you update the commit message based on my comment above? You can also implement @knikolla's comment if you'd like. Afterwards, feel free to merge the PR. Let me know if you see that you're unable to merge

part of CCI-MOC#185

Adds unit tests for MOCAPrepaidInvoice, bringing coverage from 73% to 100%.

Tests Added:

test_prepare_export : verifies that only rows where MGHPCC Managed is False are included in the export
test_output_path : verifies the output filename is correctly formatted with the invoice month
test_output_s3_key : verifies the S3 path is correctly formatted
test_output_s3_archive_key : verifies the S3 archive path is correctly formatted
Also adds new_moca_prepaid_invoice() to tests/util.py to follow the existing pattern used by all other invoices.
@marcoagonzales007 marcoagonzales007 force-pushed the add-test-mocaprepaidinvoice branch from 99cc76c to c86991c Compare June 15, 2026 17:14
@QuanMPhm QuanMPhm merged commit 78bd0d9 into CCI-MOC:main Jun 15, 2026
6 checks passed
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.

3 participants