Skip to content

Commit

Permalink
fix parameterized test duplicate function different classes (#23439)
Browse files Browse the repository at this point in the history
fixes #23434

switched parameterized function IDs to now be
`path/to/file::ClassIfExists::functionName` so it is an absolute ID and
will not be confused across classes.
  • Loading branch information
eleanorjboyd authored and DonJayamanne committed Jun 24, 2024
1 parent a8a0e59 commit b68fa75
Show file tree
Hide file tree
Showing 7 changed files with 285 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.
import pytest


class TestNotEmpty:
@pytest.mark.parametrize("a, b", [(1, 1), (2, 2)]) # test_marker--TestNotEmpty::test_integer
def test_integer(self, a, b):
assert a == b

@pytest.mark.parametrize( # test_marker--TestNotEmpty::test_string
"a, b", [("a", "a"), ("b", "b")]
)
def test_string(self, a, b):
assert a == b


class TestEmpty:
@pytest.mark.parametrize("a, b", [(0, 0)]) # test_marker--TestEmpty::test_integer
def test_integer(self, a, b):
assert a == b

@pytest.mark.parametrize("a, b", [("", "")]) # test_marker--TestEmpty::test_string
def test_string(self, a, b):
assert a == b
205 changes: 201 additions & 4 deletions python_files/tests/pytestadapter/expected_discovery_test_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@
"name": "test_adding",
"path": os.fspath(parameterize_tests_path),
"type_": "function",
"id_": "parametrize_tests.py::TestClass::test_adding",
"id_": os.fspath(parameterize_tests_path) + "::TestClass::test_adding",
"children": [
{
"name": "[3+5-8]",
Expand Down Expand Up @@ -638,7 +638,7 @@
),
},
],
"id_": "parametrize_tests.py::test_string",
"id_": os.fspath(parameterize_tests_path) + "::test_string",
},
],
},
Expand Down Expand Up @@ -760,7 +760,7 @@
),
},
],
"id_": "param_same_name/test_param1.py::test_odd_even",
"id_": os.fspath(param1_path) + "::test_odd_even",
}
],
},
Expand Down Expand Up @@ -818,7 +818,7 @@
),
},
],
"id_": "param_same_name/test_param2.py::test_odd_even",
"id_": os.fspath(param2_path) + "::test_odd_even",
}
],
},
Expand Down Expand Up @@ -1077,3 +1077,200 @@
],
"id_": str(SYMLINK_FOLDER_PATH),
}

same_function_new_class_param_expected_output = {
"name": ".data",
"path": TEST_DATA_PATH_STR,
"type_": "folder",
"children": [
{
"name": "same_function_new_class_param.py",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "file",
"id_": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"children": [
{
"name": "TestNotEmpty",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "class",
"children": [
{
"name": "test_integer",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "function",
"children": [
{
"name": "[1-1]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestNotEmpty::test_integer",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_integer[1-1]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_integer[1-1]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
{
"name": "[2-2]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestNotEmpty::test_integer",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_integer[2-2]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_integer[2-2]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
],
"id_": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py")
+ "::TestNotEmpty::test_integer",
},
{
"name": "test_string",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "function",
"children": [
{
"name": "[a-a]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestNotEmpty::test_string",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_string[a-a]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_string[a-a]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
{
"name": "[b-b]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestNotEmpty::test_string",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_string[b-b]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestNotEmpty::test_string[b-b]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
],
"id_": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py")
+ "::TestNotEmpty::test_string",
},
],
"id_": "same_function_new_class_param.py::TestNotEmpty",
},
{
"name": "TestEmpty",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "class",
"children": [
{
"name": "test_integer",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "function",
"children": [
{
"name": "[0-0]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestEmpty::test_integer",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestEmpty::test_integer[0-0]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestEmpty::test_integer[0-0]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
],
"id_": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py")
+ "::TestEmpty::test_integer",
},
{
"name": "test_string",
"path": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py"),
"type_": "function",
"children": [
{
"name": "[-]",
"path": os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
"lineno": find_test_line_number(
"TestEmpty::test_string",
os.fspath(
TEST_DATA_PATH / "same_function_new_class_param.py"
),
),
"type_": "test",
"id_": get_absolute_test_id(
"same_function_new_class_param.py::TestEmpty::test_string[-]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
"runID": get_absolute_test_id(
"same_function_new_class_param.py::TestEmpty::test_string[-]",
TEST_DATA_PATH / "same_function_new_class_param.py",
),
},
],
"id_": os.fspath(TEST_DATA_PATH / "same_function_new_class_param.py")
+ "::TestEmpty::test_string",
},
],
"id_": "same_function_new_class_param.py::TestEmpty",
},
],
}
],
"id_": TEST_DATA_PATH_STR,
}

print(param_same_name_expected_output)
10 changes: 9 additions & 1 deletion python_files/tests/pytestadapter/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ def test_parameterized_error_collect():
"test_multi_class_nest.py",
expected_discovery_test_output.nested_classes_expected_test_output,
),
(
"same_function_new_class_param.py",
expected_discovery_test_output.same_function_new_class_param_expected_output,
),
(
"test_multi_class_nest.py",
expected_discovery_test_output.nested_classes_expected_test_output,
Expand Down Expand Up @@ -187,7 +191,9 @@ def test_pytest_collect(file, expected_const):
), f"Status is not 'success', error is: {actual_item.get('error')}"
assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH)
assert is_same_tree(
actual_item.get("tests"), expected_const
actual_item.get("tests"),
expected_const,
["id_", "lineno", "name", "runID"],
), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_const, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"


Expand Down Expand Up @@ -255,6 +261,7 @@ def test_pytest_root_dir():
assert is_same_tree(
actual_item.get("tests"),
expected_discovery_test_output.root_with_config_expected_output,
["id_", "lineno", "name", "runID"],
), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_discovery_test_output.root_with_config_expected_output, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"


Expand All @@ -281,6 +288,7 @@ def test_pytest_config_file():
assert is_same_tree(
actual_item.get("tests"),
expected_discovery_test_output.root_with_config_expected_output,
["id_", "lineno", "name", "runID"],
), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_discovery_test_output.root_with_config_expected_output, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"


Expand Down
34 changes: 22 additions & 12 deletions python_files/tests/tree_comparison_helper.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,39 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.


def is_same_tree(tree1, tree2) -> bool:
"""Helper function to test if two test trees are the same.
def is_same_tree(tree1, tree2, test_key_arr, path="root") -> bool:
"""Helper function to test if two test trees are the same with detailed error logs.
`is_same_tree` starts by comparing the root attributes, and then checks if all children are the same.
"""
# Compare the root.
if any(tree1[key] != tree2[key] for key in ["path", "name", "type_"]):
return False
for key in ["path", "name", "type_", "id_"]:
if tree1.get(key) != tree2.get(key):
print(
f"Difference found at {path}: '{key}' is '{tree1.get(key)}' in tree1 and '{tree2.get(key)}' in tree2."
)
return False

# Compare child test nodes if they exist, otherwise compare test items.
if "children" in tree1 and "children" in tree2:
# sort children by path before comparing since order doesn't matter of children
# Sort children by path before comparing since order doesn't matter of children
children1 = sorted(tree1["children"], key=lambda x: x["path"])
children2 = sorted(tree2["children"], key=lambda x: x["path"])

# Compare test nodes.
if len(children1) != len(children2):
print(
f"Difference in number of children at {path}: {len(children1)} in tree1 and {len(children2)} in tree2."
)
return False
else:
return all(is_same_tree(*pair) for pair in zip(children1, children2))
for i, (child1, child2) in enumerate(zip(children1, children2)):
if not is_same_tree(child1, child2, test_key_arr, path=f"{path} -> child {i}"):
return False
elif "id_" in tree1 and "id_" in tree2:
# Compare test items.
return all(tree1[key] == tree2[key] for key in ["id_", "lineno"])
for key in test_key_arr:
if tree1.get(key) != tree2.get(key):
print(
f"Difference found at {path}: '{key}' is '{tree1.get(key)}' in tree1 and '{tree2.get(key)}' in tree2."
)
return False

return False
return True
8 changes: 5 additions & 3 deletions python_files/tests/unittestadapter/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def test_simple_discovery() -> None:
actual = discover_tests(start_dir, pattern, None)

assert actual["status"] == "success"
assert is_same_tree(actual.get("tests"), expected)
assert is_same_tree(actual.get("tests"), expected, ["id_", "lineno", "name"])
assert "error" not in actual


Expand Down Expand Up @@ -185,7 +185,7 @@ def test_simple_discovery_with_top_dir_calculated() -> None:
actual = discover_tests(start_dir, pattern, None)

assert actual["status"] == "success"
assert is_same_tree(actual.get("tests"), expected)
assert is_same_tree(actual.get("tests"), expected, ["id_", "lineno", "name"])
assert "error" not in actual


Expand Down Expand Up @@ -256,7 +256,7 @@ def test_error_discovery() -> None:
actual = discover_tests(start_dir, pattern, None)

assert actual["status"] == "error"
assert is_same_tree(expected, actual.get("tests"))
assert is_same_tree(expected, actual.get("tests"), ["id_", "lineno", "name"])
assert len(actual.get("error", [])) == 1


Expand All @@ -274,6 +274,7 @@ def test_unit_skip() -> None:
assert is_same_tree(
actual.get("tests"),
expected_discovery_test_output.skip_unittest_folder_discovery_output,
["id_", "lineno", "name"],
)
assert "error" not in actual

Expand All @@ -296,4 +297,5 @@ def test_complex_tree() -> None:
assert is_same_tree(
actual.get("tests"),
expected_discovery_test_output.complex_tree_expected_output,
["id_", "lineno", "name"],
)
Loading

0 comments on commit b68fa75

Please sign in to comment.