Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YR/-Convert-IN116-into-BC111/CIAC-10243 #4207

Merged
merged 10 commits into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion demisto_sdk/commands/validate/sdk_validation_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ select = [
"BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA114", "BA116", "BA118", "BA119", "BA124", "BA126",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA121", "PA123", "PA125", "PA127", "PA128", "PA130",
"BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC108", "BC110",
"BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC108", "BC110", "BC111",
"IN100", "IN101", "IN102", "IN104", "IN106", "IN107", "IN108", "IN109", "IN110", "IN112", "IN113", "IN114", "IN115", "IN117", "IN118", "IN121", "IN122", "IN123", "IN124", "IN125", "IN126", "IN127", "IN130",
"IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160",
"IN161", "IN162",
Expand Down
88 changes: 88 additions & 0 deletions demisto_sdk/commands/validate/tests/BC_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
from demisto_sdk.commands.validate.validators.BC_validators.BC110_new_required_argument_integration import (
NewRequiredArgumentIntegrationValidator,
)
from demisto_sdk.commands.validate.validators.BC_validators.BC111_new_required_argument_script import (
NewRequiredArgumentScriptValidator,
)
from TestSuite.repo import ChangeCWD

ALL_MARKETPLACES = list(MarketplaceVersions)
Expand Down Expand Up @@ -1224,3 +1227,88 @@ def test_NewRequiredArgumentValidator__passes():
create_old_file_pointers([new_content_item], [old_content_item])

assert not NewRequiredArgumentIntegrationValidator().is_valid([new_content_item])


@pytest.mark.parametrize(
"new_args, breaking_arg",
[
pytest.param(
[{"name": "arg1", "required": False}, {"name": "arg2", "required": True}],
"arg2",
id="new required arg",
),
pytest.param(
[
{"name": "arg1", "required": True},
],
"arg1",
id="changed to required",
),
],
)
def test_NewRequiredArgumentScriptValidator__fails(new_args, breaking_arg):
"""
Given:
- An older version of a script that has one non-required argument.
- A newer version of the same script where an argument is now required. This can occur in two cases:
Case 1: The required argument is a new addition to the script.
Case 2: An existing argument from the older version has been updated to be required in the new version.
When:
- The NewRequiredArgumentScriptValidator is invoked to validate the changes in the script.
Then:
- The validation should fail because a non-required argument has been made required. The error message should correctly identify the argument that caused the validation to fail.
"""
new_content_item = create_script_object(
paths=["args"],
values=[new_args],
)
old_content_item = create_script_object(paths=["args"], values=[[{"name": "arg1"}]])

create_old_file_pointers([new_content_item], [old_content_item])
res = NewRequiredArgumentScriptValidator().is_valid([new_content_item])
assert breaking_arg in res[0].message


@pytest.mark.parametrize(
"new_args",
[
pytest.param(
[
{"name": "arg1", "required": False},
{"name": "arg2", "required": False},
],
id="non required args",
),
pytest.param(
[
{"name": "arg1", "required": True},
{"name": "arg2", "required": True, "defaultvalue": "test"},
],
id="required with default value",
),
],
)
def test_NewRequiredArgumentScriptValidator__passes(new_args):
"""
Given:
- An older version of a script that has two arguments: one required and one not required.
- A newer version of the same script in two cases:
Case 1: Both arguments are not required.
Case 2: Both arguments are required, but the second one has a default value.
When:
- The NewRequiredArgumentScriptValidator is invoked to validate the changes in the script.
Then:
- The validation should pass in both cases:
Case 1: The first argument was changed to be not required, which is allowed.
Case 2: The first argument did not change, and the second argument was changed to be required but has a default value, which is allowed.
"""
new_content_item = create_script_object(
paths=["args"],
values=[new_args],
)
old_content_item = create_script_object(
paths=["args"], values=[[{"name": "arg1", "required": True}, {"name": "arg2"}]]
)

create_old_file_pointers([new_content_item], [old_content_item])
assert not NewRequiredArgumentScriptValidator().is_valid([new_content_item])
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from __future__ import annotations

from typing import Iterable, List

from demisto_sdk.commands.common.constants import GitStatuses
from demisto_sdk.commands.content_graph.objects.script import Script
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Script

# This validation is similar to BC110, but specifically for scripts.
class NewRequiredArgumentScriptValidator(BaseValidator[ContentTypes]):
error_code = "BC111"
description = (
"Ensure that no new *required* arguments are added to an existing script."
)
rationale = "Adding a new required argument or changing a non-required one to required without specifying a default value breaks backward compatibility."
error_message = "Possible backward compatibility break: You have added the following new *required* arguments: {arg_list}. Please undo the changes."
related_field = "script.arguments"
is_auto_fixable = False
expected_git_statuses = [GitStatuses.MODIFIED]

def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]:
results: List[ValidationResult] = []
arg_list = []
for content_item in content_items:
old_content_item = content_item.old_base_content_object

arg_list = [
arg.name
for arg in content_item.args # type: ignore
if arg.required
and not arg.defaultvalue
and (
not next(
(
old_arg
for old_arg in old_content_item.args # type: ignore
if old_arg.name == arg.name and old_arg.required
),
None,
)
)
]
if arg_list:
results.append(
ValidationResult(
validator=self,
message=self.error_message.format(
arg_list=", ".join(f'"{w}"' for w in arg_list)
),
content_object=content_item,
)
)

return results
Loading