Skip to content

Add unit tests for ColdfrontFetchProcessor#300

Open
marcoagonzales007 wants to merge 2 commits into
CCI-MOC:mainfrom
marcoagonzales007:add-coldfront-fetch-processor-tests
Open

Add unit tests for ColdfrontFetchProcessor#300
marcoagonzales007 wants to merge 2 commits into
CCI-MOC:mainfrom
marcoagonzales007:add-coldfront-fetch-processor-tests

Conversation

@marcoagonzales007

Copy link
Copy Markdown
Contributor

Part of #185

Adds additional unit tests for ColdfrontFetchProcessor bringing coverage from 76% to 82%.

Tests added:

test_get_coldfront_api_data_with_filepath — verifies that when a local filepath is provided data is read from the file instead
test_get_allocation_data_missing_key — verifies that allocation entries with missing keys are skipped

Comment on lines +274 to +277
@mock.patch(
"process_report.processors.coldfront_fetch_processor.ColdfrontFetchProcessor._fetch_coldfront_allocation_api",
)
def test_get_allocation_data_missing_key(self, mock_get_allocation_data):

@QuanMPhm QuanMPhm Jun 4, 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.

Looking at this test, it does not seem like you need any mocking at all. The test itself can be shortened.

    def test_get_allocation_data_missing_key(self):
        """Malformed allocation entries with missing keys are skipped."""
        test_allocation_data = [
            {
                "resource": {"name": "stack"},
                "project": {"pi": "PI1"},
                "attributes": {},
            },  # missing Allocated Project ID
        ]
        proc = test_utils.new_coldfront_fetch_processor()
        result = proc._get_allocation_data(test_allocation_data)
        assert result == {}

@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.

@knikolla @naved001 Looks good to me

@QuanMPhm QuanMPhm requested review from knikolla, larsks and naved001 June 11, 2026 20:58
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.

2 participants