Skip to content

Commit

Permalink
Change filenames to be OS specific paths (#3170)
Browse files Browse the repository at this point in the history 
* Change filenames to be OS specific paths
  • Loading branch information
@kddejong
kddejong authored Apr 25, 2024
1 parent eaf23ca commit b3cec6a
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 48 deletions.
25 changes: 8 additions & 17 deletions src/cfnlint/config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import os
import sys
from pathlib import Path
from typing import Dict
from typing import Dict, List, Sequence

import jsonschema

Expand Down Expand Up @@ -692,23 +692,10 @@ def templates(self):
if isinstance(filenames, str):
filenames = [filenames]

# handle different shells and Config files
# some shells don't expand * and configparser won't expand wildcards
all_filenames = []
ignore_templates = self._ignore_templates()
for filename in filenames:
add_filenames = glob.glob(filename, recursive=True)
# only way to know of the glob failed is to test it
# then add the filename as requested
if not add_filenames:
if filename not in ignore_templates:
all_filenames.append(filename)
else:
for add_filename in add_filenames:
if add_filename not in ignore_templates:
all_filenames.append(add_filename)
all_filenames = self._glob_filenames(filenames)

return sorted(all_filenames)
return [i for i in all_filenames if i not in ignore_templates]

def _ignore_templates(self):
ignore_template_args = self._get_argument_value("ignore_templates", False, True)
Expand All @@ -721,9 +708,13 @@ def _ignore_templates(self):
if isinstance(filenames, str):
filenames = [filenames]

return self._glob_filenames(filenames)

def _glob_filenames(self, filenames: Sequence[str]) -> List[str]:
# handle different shells and Config files
# some shells don't expand * and configparser won't expand wildcards
all_filenames = []

for filename in filenames:
add_filenames = glob.glob(filename, recursive=True)
# only way to know of the glob failed is to test it
Expand All @@ -733,7 +724,7 @@ def _ignore_templates(self):
else:
all_filenames.extend(add_filenames)

return all_filenames
return sorted(list(map(str, map(Path, all_filenames))))

@property
def append_rules(self):
Expand Down
7 changes: 7 additions & 0 deletions test/integration/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import subprocess
import sys
from pathlib import Path
from test.testlib.testcase import BaseTestCase

import cfnlint.core
Expand Down Expand Up @@ -33,6 +34,9 @@ def run_scenarios(self, extra_params=None):
with open(results_filename, encoding="utf-8") as json_data:
expected_results = json.load(json_data)

for result in expected_results:
result["Filename"] = str(Path(result.get("Filename")))

try:
result = subprocess.check_output(
["cfn-lint"] + extra_params + ["--format", "json", "--", filename]
Expand Down Expand Up @@ -93,6 +97,9 @@ def run_module_integration_scenarios(self, rules):
with open(results_filename, encoding="utf-8") as json_data:
expected_results = json.load(json_data)

for result in expected_results:
result["Filename"] = str(Path(result.get("Filename")))

template = cfnlint.decode.cfn_yaml.load(filename)

matches = cfnlint.core.run_checks(filename, template, rules, regions)
Expand Down
25 changes: 18 additions & 7 deletions test/integration/test_directives.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
SPDX-License-Identifier: MIT-0
"""

from pathlib import Path
from test.integration import BaseCliTestCase


Expand All @@ -11,16 +12,18 @@ class TestDirectives(BaseCliTestCase):

scenarios = [
{
"filename": "test/fixtures/templates/good/core/directives.yaml",
"filename": str(Path("test/fixtures/templates/good/core/directives.yaml")),
"exit_code": 0,
"results": [],
},
{
"filename": "test/fixtures/templates/bad/core/directives.yaml",
"filename": str(Path("test/fixtures/templates/bad/core/directives.yaml")),
"exit_code": 2,
"results": [
{
"Filename": "test/fixtures/templates/bad/core/directives.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/directives.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 18, "LineNumber": 17},
Expand All @@ -41,7 +44,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/directives.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/directives.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 13, "LineNumber": 28},
Expand All @@ -62,7 +67,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/directives.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/directives.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 16, "LineNumber": 32},
Expand All @@ -82,7 +89,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/directives.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/directives.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 13, "LineNumber": 35},
Expand All @@ -103,7 +112,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/directives.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/directives.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 15, "LineNumber": 37},
Expand Down
33 changes: 25 additions & 8 deletions test/integration/test_mandatory_checks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
SPDX-License-Identifier: MIT-0
"""

from pathlib import Path
from test.integration import BaseCliTestCase


Expand All @@ -11,11 +12,15 @@ class TestDirectives(BaseCliTestCase):

scenarios = [
{
"filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"exit_code": 2,
"results": [
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 18, "LineNumber": 17},
Expand All @@ -36,7 +41,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 13, "LineNumber": 28},
Expand All @@ -57,7 +64,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 16, "LineNumber": 32},
Expand All @@ -77,7 +86,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 13, "LineNumber": 35},
Expand All @@ -98,7 +109,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 15, "LineNumber": 37},
Expand All @@ -120,7 +133,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 18, "LineNumber": 13},
Expand All @@ -141,7 +156,9 @@ class TestDirectives(BaseCliTestCase):
},
},
{
"Filename": "test/fixtures/templates/bad/core/mandatory_checks.yaml",
"Filename": str(
Path("test/fixtures/templates/bad/core/mandatory_checks.yaml")
),
"Level": "Error",
"Location": {
"End": {"ColumnNumber": 16, "LineNumber": 19},
Expand Down
22 changes: 18 additions & 4 deletions test/unit/module/config/test_config_mixin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import logging
import os
from pathlib import Path
from test.testlib.testcase import BaseTestCase
from unittest.mock import patch

Expand Down Expand Up @@ -179,23 +180,36 @@ def test_config_expand_paths(self, yaml_mock):
self.assertEqual(
config.templates,
[
"test/fixtures/templates/public" + os.path.sep + "lambda-poller.yaml",
"test/fixtures/templates/public" + os.path.sep + "rds-cluster.yaml",
str(
Path(
"test/fixtures/templates/public"
+ os.path.sep
+ "lambda-poller.yaml"
)
),
str(
Path(
"test/fixtures/templates/public"
+ os.path.sep
+ "rds-cluster.yaml"
)
),
],
)

@patch("cfnlint.config.ConfigFileArgs._read_config", create=True)
def test_config_expand_paths_failure(self, yaml_mock):
"""Test precedence in"""

filename = "test/fixtures/templates/badpath/*.yaml"
yaml_mock.side_effect = [
{"templates": ["test/fixtures/templates/badpath/*.yaml"]},
{},
]
config = cfnlint.config.ConfigMixIn([])

# test defaults
self.assertEqual(config.templates, ["test/fixtures/templates/badpath/*.yaml"])
self.assertEqual(config.templates, [str(Path(filename))])

@patch("cfnlint.config.ConfigFileArgs._read_config", create=True)
def test_config_expand_ignore_templates(self, yaml_mock):
Expand All @@ -214,7 +228,7 @@ def test_config_expand_ignore_templates(self, yaml_mock):

# test defaults
self.assertNotIn(
"test/fixtures/templates/bad/resources/iam/resource_policy.yaml",
str(Path("test/fixtures/templates/bad/resources/iam/resource_policy.yaml")),
config.templates,
)
self.assertEqual(len(config.templates), 5)
Expand Down
17 changes: 6 additions & 11 deletions test/unit/module/core/test_run_cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""

import logging
from pathlib import Path
from test.testlib.testcase import BaseTestCase
from unittest.mock import patch

Expand Down Expand Up @@ -119,7 +120,7 @@ def test_template_via_stdin(self):
(_, filenames, _) = cfnlint.core.get_args_filenames(
["--template", filename]
)
assert filenames == [filename]
assert filenames == [str(Path(filename))]

@patch("cfnlint.config.ConfigFileArgs._read_config", create=True)
def test_template_config(self, yaml_mock):
Expand All @@ -146,9 +147,7 @@ def test_template_config(self, yaml_mock):
self.assertEqual(args.override_spec, None)
self.assertEqual(args.custom_rules, None)
self.assertEqual(args.regions, ["us-east-1"])
self.assertEqual(
args.templates, ["test/fixtures/templates/good/core/config_parameters.yaml"]
)
self.assertEqual(args.templates, [str(Path(filename))])
self.assertEqual(args.update_documentation, False)
self.assertEqual(args.update_specs, False)
self.assertEqual(args.output_file, None)
Expand Down Expand Up @@ -185,9 +184,7 @@ def test_positional_template_parameters(self, yaml_mock):
self.assertEqual(args.override_spec, None)
self.assertEqual(args.custom_rules, None)
self.assertEqual(args.regions, ["us-east-1"])
self.assertEqual(
args.templates, ["test/fixtures/templates/good/core/config_parameters.yaml"]
)
self.assertEqual(args.templates, [str(Path(filename))])
self.assertEqual(args.update_documentation, False)
self.assertEqual(args.update_specs, False)

Expand Down Expand Up @@ -217,9 +214,7 @@ def test_override_parameters(self, yaml_mock):
self.assertEqual(args.override_spec, None)
self.assertEqual(args.custom_rules, None)
self.assertEqual(args.regions, ["us-east-1"])
self.assertEqual(
args.templates, ["test/fixtures/templates/good/core/config_parameters.yaml"]
)
self.assertEqual(args.templates, [str(Path(filename))])
self.assertEqual(args.update_documentation, False)
self.assertEqual(args.update_specs, False)

Expand All @@ -243,6 +238,6 @@ def test_bad_config(self, yaml_mock):
self.assertEqual(args.override_spec, None)
self.assertEqual(args.custom_rules, None)
self.assertEqual(args.regions, ["us-east-1"])
self.assertEqual(args.templates, [filename])
self.assertEqual(args.templates, [str(Path(filename))])
self.assertEqual(args.update_documentation, False)
self.assertEqual(args.update_specs, False)
3 changes: 2 additions & 1 deletion test/unit/module/formatters/test_formatters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import sys
import xml.etree.ElementTree as ET
from pathlib import Path
from test.testlib.testcase import BaseTestCase

import jsonschema
Expand All @@ -27,7 +28,7 @@ class TestFormatters(BaseTestCase):
def setUp(self) -> None:
super().setUp()
cfnlint.core._reset_rule_cache()
self.filename = "test/fixtures/templates/bad/formatters.yaml"
self.filename = str(Path("test/fixtures/templates/bad/formatters.yaml"))
self.results = [
Match(
6,
Expand Down

0 comments on commit b3cec6a

Please sign in to comment.