From 0f23d4785aeefa0e70a0389c0384e9f30e137d7b Mon Sep 17 00:00:00 2001 From: Matthias Goerens Date: Thu, 6 Nov 2025 11:01:51 +0100 Subject: [PATCH] Fix error message for PRs with OWNERS and chart PRs containing a chart and an OWNERS file were getting the wrong error message. This was due to the check for duplicate charts, not taking into account that an OWNERS file doens't relate to a specific version, and thus considering the OWNERS file to be for a different chart. Specific tests have been added to cover this case. Fix #477 Signed-off-by: Matthias Goerens --- scripts/src/submission/submission.py | 2 +- scripts/src/submission/submission_test.py | 48 +++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/scripts/src/submission/submission.py b/scripts/src/submission/submission.py index edd31291..6443ed80 100644 --- a/scripts/src/submission/submission.py +++ b/scripts/src/submission/submission.py @@ -101,7 +101,7 @@ def register_chart_info( (self.category and self.category != category) or (self.organization and self.organization != organization) or (self.name and self.name != name) - or (self.version and self.version != version) + or (self.version and version and self.version != version) ): msg = "[ERROR] A PR must contain only one chart. Current PR includes files for multiple charts." raise DuplicateChartError(msg) diff --git a/scripts/src/submission/submission_test.py b/scripts/src/submission/submission_test.py index 1a2deaf4..9be06b6a 100644 --- a/scripts/src/submission/submission_test.py +++ b/scripts/src/submission/submission_test.py @@ -335,6 +335,33 @@ class SubmissionInitScenario: ], ), ), + # PR contains a report and an OWNERS file + # While this is an invalid PR, there shouldn't be any error during the initalization of the Submission object. + # This specific error is handled in the is_valid_certification_submission test. + # See https://github.com/openshift-helm-charts/development/issues/477 for context + SubmissionInitScenario( + api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/6", + modified_files=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS", + ], + expected_submission=submission.Submission( + api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/6", + chart=expected_chart, + modified_files=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS", + ], + report=submission.Report( + found=True, + signed=False, + path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + ), + modified_owners=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" + ], + ), + ), # PR contains additional files, not fitting into any expected category SubmissionInitScenario( api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/7", @@ -474,6 +501,27 @@ class CertificationScenario: expected_is_valid_certification=False, expected_reason="[ERROR] Send OWNERS file by itself in a separate PR.", ), + # Invalid certification Submission contains OWNERS file and report file + CertificationScenario( + input_submission=submission.Submission( + api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", + chart=expected_chart, + modified_files=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS", + ], + report=submission.Report( + found=True, + signed=False, + path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + ), + modified_owners=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" + ], + ), + expected_is_valid_certification=False, + expected_reason="[ERROR] Send OWNERS file by itself in a separate PR.", + ), # Valid certification Submission contains OWNERS and report file, but ignore_owners is set to True CertificationScenario( input_submission=submission.Submission(