Skip to content

Commit

Permalink
Improve test function names (#3852)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Oct 18, 2023
1 parent 031038e commit 83250b4
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 77 deletions.
2 changes: 1 addition & 1 deletion .config/requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ psutil # soft-dep of pytest-xdist
pylint # IDE support
pytest >= 7.2.2
pytest-mock
pytest-plus >= 0.2 # for PYTEST_REQPASS
pytest-plus >= 0.6 # for PYTEST_REQPASS
pytest-xdist >= 2.1.0
ruamel.yaml>=0.17.31,<0.18 # only the latest is expected to pass our tests
ruamel-yaml-clib # needed for mypy
Expand Down
2 changes: 1 addition & 1 deletion .config/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pylint==2.17.6
pymdown-extensions==10.3
pytest==7.4.2
pytest-mock==3.11.1
pytest-plus==0.4.0
pytest-plus==0.6.0
pytest-xdist==3.3.1
python-dateutil==2.8.2
python-slugify==8.0.1
Expand Down
7 changes: 6 additions & 1 deletion src/ansiblelint/rules/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,12 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
],
id="execution-environment-broken",
),
("examples/meta/runtime.yml", "meta-runtime", []),
pytest.param(
"examples/meta/runtime.yml",
"meta-runtime",
[],
id="meta-runtime",
),
pytest.param(
"examples/broken_collection_meta_runtime/meta/runtime.yml",
"meta-runtime",
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/__store__.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json"
},
"meta": {
"etag": "80d83d5c61044aa67496ea22c74aef08b0216c20e318400d3c939927a2761d51",
"etag": "9eb5c611e25cc9e0a180119904f8dfe39c59409b37da972f66a6c60f040a3a08",
"url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json"
},
"meta-runtime": {
Expand Down
4 changes: 2 additions & 2 deletions test/test_file_path_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@
@pytest.mark.parametrize(
"ansible_project_layout",
(
pytest.param(LAYOUT_IMPORTS, id="using only import_tasks"),
pytest.param(LAYOUT_INCLUDES, id="using only include_tasks"),
pytest.param(LAYOUT_IMPORTS, id="using-only-import_tasks"),
pytest.param(LAYOUT_INCLUDES, id="using-only-include_tasks"),
),
)
def test_file_path_evaluation(
Expand Down
26 changes: 19 additions & 7 deletions test/test_formatter_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@
@pytest.mark.parametrize(
("base_dir", "relative_path"),
(
(None, True),
("/whatever", False),
(Path("/whatever"), False),
pytest.param(None, True, id="0"),
pytest.param("/whatever", False, id="1"),
pytest.param(Path("/whatever"), False, id="2"),
),
)
@pytest.mark.parametrize(
"path",
(
pytest.param("/whatever/string", id="a"),
pytest.param(Path("/whatever/string"), id="b"),
),
)
@pytest.mark.parametrize("path", ("/whatever/string", Path("/whatever/string")))
def test_base_formatter_when_base_dir(
base_dir: Any,
relative_path: bool,
Expand All @@ -44,11 +50,17 @@ def test_base_formatter_when_base_dir(
@pytest.mark.parametrize(
"base_dir",
(
Path("/whatever"),
"/whatever",
pytest.param(Path("/whatever"), id="0"),
pytest.param("/whatever", id="1"),
),
)
@pytest.mark.parametrize(
"path",
(
pytest.param("/whatever/string", id="a"),
pytest.param(Path("/whatever/string"), id="b"),
),
)
@pytest.mark.parametrize("path", ("/whatever/string", Path("/whatever/string")))
def test_base_formatter_when_base_dir_is_given_and_relative_is_true(
path: str | Path,
base_dir: str | Path,
Expand Down
4 changes: 2 additions & 2 deletions test/test_formatter_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def setup_class(self) -> None:
display_relative_path=True,
)

def test_format_list(self) -> None:
def test_json_format_list(self) -> None:
"""Test if the return value is a string."""
assert isinstance(self.formatter, CodeclimateJSONFormatter)
assert isinstance(self.formatter.format_result(self.matches), str)
Expand All @@ -66,7 +66,7 @@ def test_result_is_json(self) -> None:
# https://github.com/ansible/ansible-navigator/issues/1490
assert "\n" not in output

def test_single_match(self) -> None:
def test_json_single_match(self) -> None:
"""Test negative case. Only lists are allowed. Otherwise a RuntimeError will be raised."""
assert isinstance(self.formatter, CodeclimateJSONFormatter)
with pytest.raises(RuntimeError):
Expand Down
12 changes: 6 additions & 6 deletions test/test_formatter_sarif.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,20 @@ def setup_class(self) -> None:

self.formatter = SarifFormatter(pathlib.Path.cwd(), display_relative_path=True)

def test_format_list(self) -> None:
def test_sarif_format_list(self) -> None:
"""Test if the return value is a string."""
assert isinstance(self.formatter, SarifFormatter)
assert isinstance(self.formatter.format_result(self.matches), str)

def test_result_is_json(self) -> None:
def test_sarif_result_is_json(self) -> None:
"""Test if returned string value is a JSON."""
assert isinstance(self.formatter, SarifFormatter)
output = self.formatter.format_result(self.matches)
json.loads(output)
# https://github.com/ansible/ansible-navigator/issues/1490
assert "\n" not in output

def test_single_match(self) -> None:
def test_sarif_single_match(self) -> None:
"""Test negative case. Only lists are allowed. Otherwise, a RuntimeError will be raised."""
assert isinstance(self.formatter, SarifFormatter)
with pytest.raises(RuntimeError):
Expand Down Expand Up @@ -182,8 +182,8 @@ def test_sarif_parsable_ignored() -> None:
@pytest.mark.parametrize(
("file", "return_code"),
(
pytest.param("examples/playbooks/valid.yml", 0),
pytest.param("playbook.yml", 2),
pytest.param("examples/playbooks/valid.yml", 0, id="0"),
pytest.param("playbook.yml", 2, id="1"),
),
)
def test_sarif_file(file: str, return_code: int) -> None:
Expand All @@ -204,7 +204,7 @@ def test_sarif_file(file: str, return_code: int) -> None:

@pytest.mark.parametrize(
("file", "return_code"),
(pytest.param("examples/playbooks/valid.yml", 0),),
(pytest.param("examples/playbooks/valid.yml", 0, id="0"),),
)
def test_sarif_file_creates_it_if_none_exists(file: str, return_code: int) -> None:
"""Test ability to create sarif file if none exists and dump output to it (--sarif-file)."""
Expand Down
6 changes: 3 additions & 3 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ def test_call_from_outside_venv(expected_warning: bool) -> None:
@pytest.mark.parametrize(
("ver_diff", "found", "check", "outlen"),
(
("v1.2.2", True, "pre-release", 1),
("v1.2.3", False, "", 1),
("v1.2.4", True, "new release", 2),
pytest.param("v1.2.2", True, "pre-release", 1, id="0"),
pytest.param("v1.2.3", False, "", 1, id="1"),
pytest.param("v1.2.4", True, "new release", 2, id="2"),
),
)
def test_get_version_warning(
Expand Down
36 changes: 17 additions & 19 deletions test/test_matcherrror.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,18 @@ def test_match_error_not_equal(
@pytest.mark.parametrize(
"other",
(
None,
"foo",
42,
Exception("foo"),
pytest.param(None, id="none"),
pytest.param("foo", id="str"),
pytest.param(42, id="int"),
pytest.param(Exception("foo"), id="exc"),
),
ids=repr,
)
@pytest.mark.parametrize(
("operation", "operator_char"),
(
pytest.param(operator.le, "<=", id="<="),
pytest.param(operator.gt, ">", id=">"),
pytest.param(operator.le, "<=", id="le"),
pytest.param(operator.gt, ">", id="gt"),
),
)
def test_matcherror_compare_no_other_fallback(
Expand All @@ -154,21 +154,20 @@ def test_matcherror_compare_no_other_fallback(
@pytest.mark.parametrize(
"other",
(
None,
"foo",
42,
Exception("foo"),
DummyTestObject(),
pytest.param(None, id="none"),
pytest.param("foo", id="str"),
pytest.param(42, id="int"),
pytest.param(Exception("foo"), id="exception"),
pytest.param(DummyTestObject(), id="obj"),
),
ids=repr,
)
@pytest.mark.parametrize(
("operation", "expected_value"),
(
(operator.eq, False),
(operator.ne, True),
pytest.param(operator.eq, False, id="eq"),
pytest.param(operator.ne, True, id="ne"),
),
ids=("==", "!="),
)
def test_matcherror_compare_with_other_fallback(
other: object,
Expand All @@ -182,16 +181,15 @@ def test_matcherror_compare_with_other_fallback(
@pytest.mark.parametrize(
("operation", "expected_value"),
(
(operator.eq, "EQ_SENTINEL"),
(operator.ne, "NE_SENTINEL"),
pytest.param(operator.eq, "EQ_SENTINEL", id="eq"),
pytest.param(operator.ne, "NE_SENTINEL", id="ne"),
# NOTE: these are swapped because when we do `x < y`, and `x.__lt__(y)`
# NOTE: returns `NotImplemented`, Python will reverse the check into
# NOTE: `y > x`, and so `y.__gt__(x) is called.
# Ref: https://docs.python.org/3/reference/datamodel.html#object.__lt__
(operator.lt, "GT_SENTINEL"),
(operator.gt, "LT_SENTINEL"),
pytest.param(operator.lt, "GT_SENTINEL", id="gt"),
pytest.param(operator.gt, "LT_SENTINEL", id="lt"),
),
ids=("==", "!=", "<", ">"),
)
def test_matcherror_compare_with_dummy_sentinel(
operation: Callable[..., bool],
Expand Down
5 changes: 4 additions & 1 deletion test/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ def test_runner_exclude_paths(default_rules_collection: RulesCollection) -> None
assert len(matches) == 0


@pytest.mark.parametrize(("exclude_path"), ("**/playbooks/*.yml",))
@pytest.mark.parametrize(
("exclude_path"),
(pytest.param("**/playbooks/*.yml", id="1"),),
)
def test_runner_exclude_globs(
default_rules_collection: RulesCollection,
exclude_path: str,
Expand Down
4 changes: 2 additions & 2 deletions test/test_skiputils.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
@pytest.mark.parametrize(
("line", "expected"),
(
("foo # noqa: bar", "bar"),
("foo # noqa bar", "bar"),
pytest.param("foo # noqa: bar", "bar", id="0"),
pytest.param("foo # noqa bar", "bar", id="1"),
),
)
def test_get_rule_skips_from_line(line: str, expected: str) -> None:
Expand Down
81 changes: 50 additions & 31 deletions test/test_transform_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,72 +55,91 @@ def test_seek_with_bad_path(
@pytest.mark.parametrize(
("yaml_path", "data", "expected"),
(
([], DUMMY_MAP, DUMMY_MAP),
(["foo"], DUMMY_MAP, DUMMY_MAP["foo"]),
(["bar"], DUMMY_MAP, DUMMY_MAP["bar"]),
(["bar", "some"], DUMMY_MAP, DUMMY_MAP["bar"]["some"]),
(["fruits"], DUMMY_MAP, DUMMY_MAP["fruits"]),
(["fruits", 0], DUMMY_MAP, DUMMY_MAP["fruits"][0]),
(["fruits", 1], DUMMY_MAP, DUMMY_MAP["fruits"][1]),
(["answer"], DUMMY_MAP, DUMMY_MAP["answer"]),
(["answer", 0], DUMMY_MAP, DUMMY_MAP["answer"][0]),
(["answer", 0, "forty-two"], DUMMY_MAP, DUMMY_MAP["answer"][0]["forty-two"]),
(
pytest.param([], DUMMY_MAP, DUMMY_MAP, id="0"),
pytest.param(["foo"], DUMMY_MAP, DUMMY_MAP["foo"], id="1"),
pytest.param(["bar"], DUMMY_MAP, DUMMY_MAP["bar"], id="2"),
pytest.param(["bar", "some"], DUMMY_MAP, DUMMY_MAP["bar"]["some"], id="3"),
pytest.param(["fruits"], DUMMY_MAP, DUMMY_MAP["fruits"], id="4"),
pytest.param(["fruits", 0], DUMMY_MAP, DUMMY_MAP["fruits"][0], id="5"),
pytest.param(["fruits", 1], DUMMY_MAP, DUMMY_MAP["fruits"][1], id="6"),
pytest.param(["answer"], DUMMY_MAP, DUMMY_MAP["answer"], id="7"),
pytest.param(["answer", 0], DUMMY_MAP, DUMMY_MAP["answer"][0], id="8"),
pytest.param(
["answer", 0, "forty-two"],
DUMMY_MAP,
DUMMY_MAP["answer"][0]["forty-two"],
id="9",
),
pytest.param(
["answer", 0, "forty-two", 0],
DUMMY_MAP,
DUMMY_MAP["answer"][0]["forty-two"][0],
id="10",
),
(
pytest.param(
["answer", 0, "forty-two", 1],
DUMMY_MAP,
DUMMY_MAP["answer"][0]["forty-two"][1],
id="11",
),
(
pytest.param(
["answer", 0, "forty-two", 2],
DUMMY_MAP,
DUMMY_MAP["answer"][0]["forty-two"][2],
id="12",
),
pytest.param([], DUMMY_LIST, DUMMY_LIST, id="13"),
pytest.param([0], DUMMY_LIST, DUMMY_LIST[0], id="14"),
pytest.param([0, "foo"], DUMMY_LIST, DUMMY_LIST[0]["foo"], id="15"),
pytest.param([1], DUMMY_LIST, DUMMY_LIST[1], id="16"),
pytest.param([1, "bar"], DUMMY_LIST, DUMMY_LIST[1]["bar"], id="17"),
pytest.param(
[1, "bar", "some"],
DUMMY_LIST,
DUMMY_LIST[1]["bar"]["some"],
id="18",
),
([], DUMMY_LIST, DUMMY_LIST),
([0], DUMMY_LIST, DUMMY_LIST[0]),
([0, "foo"], DUMMY_LIST, DUMMY_LIST[0]["foo"]),
([1], DUMMY_LIST, DUMMY_LIST[1]),
([1, "bar"], DUMMY_LIST, DUMMY_LIST[1]["bar"]),
([1, "bar", "some"], DUMMY_LIST, DUMMY_LIST[1]["bar"]["some"]),
([1, "fruits"], DUMMY_LIST, DUMMY_LIST[1]["fruits"]),
([1, "fruits", 0], DUMMY_LIST, DUMMY_LIST[1]["fruits"][0]),
([1, "fruits", 1], DUMMY_LIST, DUMMY_LIST[1]["fruits"][1]),
([2], DUMMY_LIST, DUMMY_LIST[2]),
([2, "answer"], DUMMY_LIST, DUMMY_LIST[2]["answer"]),
([2, "answer", 0], DUMMY_LIST, DUMMY_LIST[2]["answer"][0]),
(
pytest.param([1, "fruits"], DUMMY_LIST, DUMMY_LIST[1]["fruits"], id="19"),
pytest.param([1, "fruits", 0], DUMMY_LIST, DUMMY_LIST[1]["fruits"][0], id="20"),
pytest.param([1, "fruits", 1], DUMMY_LIST, DUMMY_LIST[1]["fruits"][1], id="21"),
pytest.param([2], DUMMY_LIST, DUMMY_LIST[2], id="22"),
pytest.param([2, "answer"], DUMMY_LIST, DUMMY_LIST[2]["answer"], id="23"),
pytest.param([2, "answer", 0], DUMMY_LIST, DUMMY_LIST[2]["answer"][0], id="24"),
pytest.param(
[2, "answer", 0, "forty-two"],
DUMMY_LIST,
DUMMY_LIST[2]["answer"][0]["forty-two"],
id="25",
),
(
pytest.param(
[2, "answer", 0, "forty-two", 0],
DUMMY_LIST,
DUMMY_LIST[2]["answer"][0]["forty-two"][0],
id="26",
),
(
pytest.param(
[2, "answer", 0, "forty-two", 1],
DUMMY_LIST,
DUMMY_LIST[2]["answer"][0]["forty-two"][1],
id="27",
),
(
pytest.param(
[2, "answer", 0, "forty-two", 2],
DUMMY_LIST,
DUMMY_LIST[2]["answer"][0]["forty-two"][2],
id="28",
),
(
pytest.param(
[],
"this is a string that should be returned as is, ignoring path.",
"this is a string that should be returned as is, ignoring path.",
id="29",
),
(
pytest.param(
[2, "answer", 0, "forty-two", 2],
"this is a string that should be returned as is, ignoring path.",
"this is a string that should be returned as is, ignoring path.",
id="30",
),
),
)
Expand Down

0 comments on commit 83250b4

Please sign in to comment.