Skip to content

Conversation

@joao-faria-dev
Copy link
Contributor

@joao-faria-dev joao-faria-dev commented Nov 11, 2025

Related issue:

Description

Add test coverage for the helpers functions on process_submissions.py

@joao-faria-dev joao-faria-dev force-pushed the 1611-add-process-submissions-tests branch 3 times, most recently from c012639 to 826632b Compare November 11, 2025 21:32
@MarceloRobert MarceloRobert added CI/CD Most or all of the changes for this issue will be in in GitHub Actions Ingester The issue relates to the ingester tool, including the command itself and related functions. labels Nov 12, 2025
Copy link
Contributor

@gustavobtflores gustavobtflores left a comment

Choose a reason for hiding this comment

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

Looks great overall, just some minor comments

Comment on lines 448 to 474
@patch("kernelCI_app.management.commands.helpers.process_submissions.logger")
@patch(
"kernelCI_app.management.commands.helpers.process_submissions.make_issue_instance"
)
def test_insert_items_with_errors(self, mock_make_issue, mock_logger):
items = [{"id": "issue", "version": 1, "origin": "test"}]

mock_make_issue.side_effect = ValidationError.from_exception_data(
"TestModel", []
)
result = insert_items("issues", items)
self.assertEqual(result, 0)
self.assertEqual(mock_logger.error.call_count, 1)

mock_make_issue.reset_mock()
mock_logger.reset_mock()

mock_issue = MagicMock()
mock_issue.save = MagicMock(side_effect=IntegrityError("Duplicate key"))
mock_make_issue.return_value = mock_issue
result = insert_items("issues", items)
self.assertEqual(result, 0)
self.assertEqual(mock_logger.error.call_count, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

could we separate this test in two different cases?

1 - ValidationError
2 - IntegrityError

Comment on lines +426 to +430
class TestInsertItems(SimpleTestCase):
@patch("kernelCI_app.management.commands.helpers.process_submissions.logger")
@patch(
"kernelCI_app.management.commands.helpers.process_submissions.make_issue_instance"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

we could add a test case with an empty items list in this class, just in case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, applied both comments. Can you check pls?

@joao-faria-dev joao-faria-dev force-pushed the 1611-add-process-submissions-tests branch from 826632b to 9028eea Compare November 12, 2025 18:10
Copy link
Contributor

@gustavobtflores gustavobtflores left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work!

Could you please just rebase the branch with main? I will merge it then

@joao-faria-dev joao-faria-dev force-pushed the 1611-add-process-submissions-tests branch from 9028eea to b2cc252 Compare November 12, 2025 19:23
@joao-faria-dev
Copy link
Contributor Author

Just rebased @gustavobtflores

@gustavobtflores gustavobtflores added this pull request to the merge queue Nov 12, 2025
Merged via the queue into kernelci:main with commit ca5d24d Nov 12, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Most or all of the changes for this issue will be in in GitHub Actions Ingester The issue relates to the ingester tool, including the command itself and related functions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create unit tests for ingester's process submission file

3 participants