Skip to content

Commit

Permalink
BA125 validation (#4161)
Browse files Browse the repository at this point in the history
* new format

* casefold moved

* renamed

* first run

* add release notes to packs

* a tiny bit prettier

* added tests for ba125

* readme_content for tests

* added ContentTypes pack

* added path for the error message

* format

* fixed conflict

* format

* format

* conflict fixed

* added BA125 to use_git

* Update demisto_sdk/commands/validate/validators/BA_validators/BA125_customer_facing_docs_disallowed_terms.py

Co-authored-by: Yuval Hayun <70104171+YuvHayun@users.noreply.github.com>

* Update demisto_sdk/commands/validate/validators/BA_validators/BA125_customer_facing_docs_disallowed_terms.py

Co-authored-by: Yuval Hayun <70104171+YuvHayun@users.noreply.github.com>

* Update demisto_sdk/commands/validate/tests/test_tools.py

Co-authored-by: Yuval Hayun <70104171+YuvHayun@users.noreply.github.com>

* mypy

---------

Co-authored-by: dorschw <81086590+dorschw@users.noreply.github.com>
Co-authored-by: Yuval Hayun <70104171+YuvHayun@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 2, 2024
1 parent f33937b commit d2ce3b5
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 1 deletion.
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 @@ -22,7 +22,7 @@ ignorable_errors = [

[use_git]
select = [
"BA100", "BA101", "BA105", "BA106", "BA108", "BA110", "BA111", "BA114", "BA116", "BA118", "BA119", "BA124", "BA126",
"BA100", "BA101", "BA105", "BA106", "BA108", "BA110", "BA111", "BA114", "BA116", "BA118", "BA119", "BA124", "BA125", "BA126",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA121", "PA123", "PA125", "PA127", "PA128", "PA130", "PA131", "PA132",
"BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC108", "BC110", "BC111",
Expand Down
73 changes: 73 additions & 0 deletions demisto_sdk/commands/validate/tests/BA_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@
from demisto_sdk.commands.validate.validators.BA_validators.BA119_is_py_file_contain_copy_right_section import (
IsPyFileContainCopyRightSectionValidator,
)
from demisto_sdk.commands.validate.validators.BA_validators.BA125_customer_facing_docs_disallowed_terms import (
CustomerFacingDocsDisallowedTermsValidator,
)
from demisto_sdk.commands.validate.validators.BA_validators.BA126_content_item_is_deprecated_correctly import (
IsDeprecatedCorrectlyValidator,
)
Expand Down Expand Up @@ -1573,6 +1576,76 @@ def test_ValidPackNameValidator_is_valid(
)


@pytest.mark.parametrize(
"content_items, expected_number_of_failures, expected_error_message",
[
pytest.param([create_integration_object()], 0, "", id="valid: integration"),
pytest.param(
[create_integration_object(readme_content="test-module")],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_233/Integrations/integration_0/README.md",
id="invalid: integration readme",
),
pytest.param(
[create_integration_object(description_content="test-module")],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_234/Integrations/integration_0/integration_0_description.md",
id="invalid: integration description",
),
pytest.param([create_script_object()], 0, "", id="valid: script"),
pytest.param(
[create_script_object(readme_content="test-module ")],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_236/Scripts/script0/README.md",
id="invalid: script readme",
),
pytest.param([create_playbook_object()], 0, "", id="valid: playbook"),
pytest.param(
[create_playbook_object(readme_content="test-module ")],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_238/Playbooks/playbook-0_README.md",
id="invalid: playbook readme",
),
pytest.param([create_pack_object()], 0, "", id="valid: pack"),
pytest.param(
[create_pack_object(readme_text="test-module ")],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_240/README.md",
id="invalid: pack readme",
),
pytest.param(
[
create_pack_object(
["version"], ["1.0.1"], release_note_content="test-module"
)
],
1,
"Found internal terms in a customer-facing documentation: found test-module in Packs/pack_241/ReleaseNotes/1_0_1.md",
id="invalid: pack release note",
),
],
)
def test_CustomerFacingDocsDisallowedTermsValidator(
content_items, expected_number_of_failures, expected_error_message
):
"""
Given
- Case 1: Content items containing disallowed terms in their related files.
- Case 2: Content items containing only valid terms in their related files.
When
- Running the CustomerFacingDocsDisallowedTermsValidator validation.
Then
- Case 1: Fail the validation with a relevant message containing the found disallowed terms.
- Case 2: Don't fail the validation.
"""
results = CustomerFacingDocsDisallowedTermsValidator().is_valid(
content_items=content_items
)
assert len(results) == expected_number_of_failures
if results:
assert results[0].message == expected_error_message


@pytest.mark.parametrize(
"content_items, expected_number_of_failures, expected_msgs",
[
Expand Down
10 changes: 10 additions & 0 deletions demisto_sdk/commands/validate/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def create_integration_object(
values: Optional[List[Any]] = None,
pack_info: Optional[Dict[str, Any]] = None,
readme_content: Optional[str] = None,
description_content: Optional[str] = None,
name: Optional[str] = None,
code: Optional[str] = None,
) -> Integration:
Expand All @@ -75,12 +76,16 @@ def create_integration_object(
"""
yml_content = load_yaml("integration.yml")
update_keys(yml_content, paths, values)

pack = REPO.create_pack()
if pack_info:
pack.set_data(**pack_info)

additional_params = {}

if description_content:
additional_params["description"] = description_content

if readme_content is not None:
additional_params["readme"] = readme_content

Expand Down Expand Up @@ -217,6 +222,7 @@ def create_script_object(
paths: Optional[List[str]] = None,
values: Optional[List[Any]] = None,
pack_info: Optional[Dict[str, Any]] = None,
readme_content: Optional[str] = None,
name: Optional[str] = None,
code: Optional[str] = None,
test_code: Optional[str] = None,
Expand All @@ -240,6 +246,9 @@ def create_script_object(
pack = REPO.create_pack()
if pack_info:
pack.set_data(**pack_info)
if readme_content is not None:
additional_params["readme"] = readme_content

script = pack.create_script(yml=yml_content, **additional_params)
code = code or "from MicrosoftApiModule import *"
script.code.write(code)
Expand Down Expand Up @@ -288,6 +297,7 @@ def create_pack_object(
PackParser.parse_ignored_errors = MagicMock(return_value={})
pack.pack_metadata.write_json(json_content)
pack.readme.write_text(readme_text)

if image is not None:
pack.author_image.write(image)
if playbooks:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
from __future__ import annotations

from typing import Iterable, List, Union

from demisto_sdk.commands.common.constants import GitStatuses
from demisto_sdk.commands.content_graph.objects.integration import Integration
from demisto_sdk.commands.content_graph.objects.pack import Pack
from demisto_sdk.commands.content_graph.objects.playbook import Playbook
from demisto_sdk.commands.content_graph.objects.script import Script
from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Union[Integration, Script, Playbook, Pack]

disallowed_terms = [ # These terms are checked regardless for case (case-insensitive)
"test-module",
"test module",
"long-running-execution",
]


class CustomerFacingDocsDisallowedTermsValidator(BaseValidator[ContentTypes]):
error_code = "BA125"
description = "Validate that customer facing docs and fields don't contain any internal terms that aren't clear for customers."
rationale = "Ensure customer-facing docs avoid internal terms for clarity."
error_message = (
"Found internal terms in a customer-facing documentation: found {terms}"
)
related_field = ""
is_auto_fixable = False
related_file_type = [
RelatedFileType.README,
RelatedFileType.DESCRIPTION_File,
RelatedFileType.RELEASE_NOTE,
]
expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED]

def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]:
found_terms: dict[str, str] = {}
return [
ValidationResult(
validator=self,
message=self.error_message.format(
terms=self.format_error_message(found_terms)
),
content_object=content_item,
)
for content_item in content_items
if self.find_disallowed_terms(
self.get_related_files(content_item), found_terms
)
]

def format_error_message(self, found_terms):
return "\n".join(
f"{', '.join(found_terms[file])} in {file}" for file in found_terms
)

def find_disallowed_terms(self, related_files, found_terms):
for file in related_files:
file_content = file.file_content.casefold()
terms = [term for term in disallowed_terms if term in file_content]
if terms:
# Extract the filename from the file path
filename = "/".join(
file.file_path.parts[file.file_path.parts.index("Packs") :]
)
found_terms[f"{filename}"] = terms
return found_terms

def get_related_files(self, content_item):
related_files = [content_item.readme]
if isinstance(content_item, Integration):
related_files.append(content_item.description_file)
elif isinstance(content_item, Pack):
related_files.append(content_item.release_note)
return related_files

0 comments on commit d2ce3b5

Please sign in to comment.