From 6a51d275b31954985f19330bcdf9aa1db92080fd Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 10 Jul 2023 11:06:42 -0700 Subject: [PATCH 1/2] fix duplicate class names in explorer --- .../.data/unittest_folder/test_add.py | 8 + .../.data/unittest_folder/test_subtract.py | 8 + .../expected_discovery_test_output.py | 47 ++++- .../tests/pytestadapter/test_discovery.py | 185 +++++++++--------- pythonFiles/vscode_pytest/__init__.py | 4 +- 5 files changed, 155 insertions(+), 97 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_add.py b/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_add.py index a96c7f2fa392..e9bdda0ad2ad 100644 --- a/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_add.py +++ b/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_add.py @@ -19,3 +19,11 @@ def test_add_positive_numbers(self): # test_marker--test_add_positive_numbers def test_add_negative_numbers(self): # test_marker--test_add_negative_numbers result = add(-2, -3) self.assertEqual(result, -5) + + +class TestDuplicateFunction(unittest.TestCase): + # This test's id is unittest_folder/test_subtract.py::TestDuplicateFunction::test_dup_a. It has the same class name as + # another test, but it's in a different file, so it should not be confused. + # This test passes. + def test_dup_a(self): # test_marker--test_dup_a + self.assertEqual(1, 1) diff --git a/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_subtract.py b/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_subtract.py index 087e5140def4..634a6d81f9eb 100644 --- a/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_subtract.py +++ b/pythonFiles/tests/pytestadapter/.data/unittest_folder/test_subtract.py @@ -24,3 +24,11 @@ def test_subtract_negative_numbers( # test_marker--test_subtract_negative_numbe result = subtract(-2, -3) # This is intentional to test assertion failures self.assertEqual(result, 100000) + + +class TestDuplicateFunction(unittest.TestCase): + # This test's id is unittest_folder/test_subtract.py::TestDuplicateFunction::test_dup_s. It has the same class name as + # another test, but it's in a different file, so it should not be confused. + # This test passes. + def test_dup_s(self): # test_marker--test_dup_s + self.assertEqual(1, 1) diff --git a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index 0f733c94d141..b05ed5e9f00c 100644 --- a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -106,10 +106,14 @@ # │ └── TestAddFunction # │ ├── test_add_negative_numbers # │ └── test_add_positive_numbers +# │ └── TestDuplicateFunction +# │ └── test_dup_a # └── test_subtract.py # └── TestSubtractFunction # ├── test_subtract_negative_numbers # └── test_subtract_positive_numbers +# │ └── TestDuplicateFunction +# │ └── test_dup_s unittest_folder_path = os.fspath(TEST_DATA_PATH / "unittest_folder") test_add_path = os.fspath(TEST_DATA_PATH / "unittest_folder" / "test_add.py") test_subtract_path = os.fspath(TEST_DATA_PATH / "unittest_folder" / "test_subtract.py") @@ -159,7 +163,26 @@ }, ], "id_": "unittest_folder/test_add.py::TestAddFunction", - } + }, + { + "name": "TestDuplicateFunction", + "path": test_add_path, + "type_": "class", + "children": [ + { + "name": "test_dup_a", + "path": test_add_path, + "lineno": find_test_line_number( + "test_dup_a", + test_add_path, + ), + "type_": "test", + "id_": "unittest_folder/test_add.py::TestDuplicateFunction::test_dup_a", + "runID": "unittest_folder/test_add.py::TestDuplicateFunction::test_dup_a", + }, + ], + "id_": "unittest_folder/test_add.py::TestDuplicateFunction", + }, ], }, { @@ -197,7 +220,26 @@ }, ], "id_": "unittest_folder/test_subtract.py::TestSubtractFunction", - } + }, + { + "name": "TestDuplicateFunction", + "path": test_subtract_path, + "type_": "class", + "children": [ + { + "name": "test_dup_s", + "path": test_subtract_path, + "lineno": find_test_line_number( + "test_dup_s", + test_subtract_path, + ), + "type_": "test", + "id_": "unittest_folder/test_subtract.py::TestDuplicateFunction::test_dup_s", + "runID": "unittest_folder/test_subtract.py::TestDuplicateFunction::test_dup_s", + }, + ], + "id_": "unittest_folder/test_subtract.py::TestDuplicateFunction", + }, ], }, ], @@ -206,6 +248,7 @@ "id_": TEST_DATA_PATH_STR, } + # This is the expected output for the dual_level_nested_folder tests # └── dual_level_nested_folder # └── test_top_folder.py diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 02ea1ddcd871..d4bf7a32867a 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -9,104 +9,103 @@ from . import expected_discovery_test_output from .helpers import TEST_DATA_PATH, runner - -def test_import_error(tmp_path): - """Test pytest discovery on a file that has a pytest marker but does not import pytest. - - Copies the contents of a .txt file to a .py file in the temporary directory - to then run pytest discovery on. - - The json should still be returned but the errors list should be present. - - Keyword arguments: - tmp_path -- pytest fixture that creates a temporary directory. - """ - # Saving some files as .txt to avoid that file displaying a syntax error for - # the extension as a whole. Instead, rename it before running this test - # in order to test the error handling. - file_path = TEST_DATA_PATH / "error_pytest_import.txt" - temp_dir = tmp_path / "temp_data" - temp_dir.mkdir() - p = temp_dir / "error_pytest_import.py" - shutil.copyfile(file_path, p) - actual_list: Optional[List[Dict[str, Any]]] = runner( - ["--collect-only", os.fspath(p)] - ) - assert actual_list - for actual in actual_list: - assert all(item in actual for item in ("status", "cwd", "error")) - assert actual["status"] == "error" - assert actual["cwd"] == os.fspath(TEST_DATA_PATH) - assert len(actual["error"]) == 2 - - -def test_syntax_error(tmp_path): - """Test pytest discovery on a file that has a syntax error. - - Copies the contents of a .txt file to a .py file in the temporary directory - to then run pytest discovery on. - - The json should still be returned but the errors list should be present. - - Keyword arguments: - tmp_path -- pytest fixture that creates a temporary directory. - """ - # Saving some files as .txt to avoid that file displaying a syntax error for - # the extension as a whole. Instead, rename it before running this test - # in order to test the error handling. - file_path = TEST_DATA_PATH / "error_syntax_discovery.txt" - temp_dir = tmp_path / "temp_data" - temp_dir.mkdir() - p = temp_dir / "error_syntax_discovery.py" - shutil.copyfile(file_path, p) - actual = runner(["--collect-only", os.fspath(p)]) - if actual: - actual = actual[0] - assert actual - assert all(item in actual for item in ("status", "cwd", "error")) - assert actual["status"] == "error" - assert actual["cwd"] == os.fspath(TEST_DATA_PATH) - assert len(actual["error"]) == 2 - - -def test_parameterized_error_collect(): - """Tests pytest discovery on specific file that incorrectly uses parametrize. - - The json should still be returned but the errors list should be present. - """ - file_path_str = "error_parametrize_discovery.py" - actual = runner(["--collect-only", file_path_str]) - if actual: - actual = actual[0] - assert all(item in actual for item in ("status", "cwd", "error")) - assert actual["status"] == "error" - assert actual["cwd"] == os.fspath(TEST_DATA_PATH) - assert len(actual["error"]) == 2 +# def test_import_error(tmp_path): +# """Test pytest discovery on a file that has a pytest marker but does not import pytest. + +# Copies the contents of a .txt file to a .py file in the temporary directory +# to then run pytest discovery on. + +# The json should still be returned but the errors list should be present. + +# Keyword arguments: +# tmp_path -- pytest fixture that creates a temporary directory. +# """ +# # Saving some files as .txt to avoid that file displaying a syntax error for +# # the extension as a whole. Instead, rename it before running this test +# # in order to test the error handling. +# file_path = TEST_DATA_PATH / "error_pytest_import.txt" +# temp_dir = tmp_path / "temp_data" +# temp_dir.mkdir() +# p = temp_dir / "error_pytest_import.py" +# shutil.copyfile(file_path, p) +# actual_list: Optional[List[Dict[str, Any]]] = runner( +# ["--collect-only", os.fspath(p)] +# ) +# assert actual_list +# for actual in actual_list: +# assert all(item in actual for item in ("status", "cwd", "error")) +# assert actual["status"] == "error" +# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) +# assert len(actual["error"]) == 2 + + +# def test_syntax_error(tmp_path): +# """Test pytest discovery on a file that has a syntax error. + +# Copies the contents of a .txt file to a .py file in the temporary directory +# to then run pytest discovery on. + +# The json should still be returned but the errors list should be present. + +# Keyword arguments: +# tmp_path -- pytest fixture that creates a temporary directory. +# """ +# # Saving some files as .txt to avoid that file displaying a syntax error for +# # the extension as a whole. Instead, rename it before running this test +# # in order to test the error handling. +# file_path = TEST_DATA_PATH / "error_syntax_discovery.txt" +# temp_dir = tmp_path / "temp_data" +# temp_dir.mkdir() +# p = temp_dir / "error_syntax_discovery.py" +# shutil.copyfile(file_path, p) +# actual = runner(["--collect-only", os.fspath(p)]) +# if actual: +# actual = actual[0] +# assert actual +# assert all(item in actual for item in ("status", "cwd", "error")) +# assert actual["status"] == "error" +# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) +# assert len(actual["error"]) == 2 + + +# def test_parameterized_error_collect(): +# """Tests pytest discovery on specific file that incorrectly uses parametrize. + +# The json should still be returned but the errors list should be present. +# """ +# file_path_str = "error_parametrize_discovery.py" +# actual = runner(["--collect-only", file_path_str]) +# if actual: +# actual = actual[0] +# assert all(item in actual for item in ("status", "cwd", "error")) +# assert actual["status"] == "error" +# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) +# assert len(actual["error"]) == 2 @pytest.mark.parametrize( "file, expected_const", [ - ( - "param_same_name", - expected_discovery_test_output.param_same_name_expected_output, - ), - ( - "parametrize_tests.py", - expected_discovery_test_output.parametrize_tests_expected_output, - ), - ( - "empty_discovery.py", - expected_discovery_test_output.empty_discovery_pytest_expected_output, - ), - ( - "simple_pytest.py", - expected_discovery_test_output.simple_discovery_pytest_expected_output, - ), - ( - "unittest_pytest_same_file.py", - expected_discovery_test_output.unit_pytest_same_file_discovery_expected_output, - ), + # ( + # "param_same_name", + # expected_discovery_test_output.param_same_name_expected_output, + # ), + # ( + # "parametrize_tests.py", + # expected_discovery_test_output.parametrize_tests_expected_output, + # ), + # ( + # "empty_discovery.py", + # expected_discovery_test_output.empty_discovery_pytest_expected_output, + # ), + # ( + # "simple_pytest.py", + # expected_discovery_test_output.simple_discovery_pytest_expected_output, + # ), + # ( + # "unittest_pytest_same_file.py", + # expected_discovery_test_output.unit_pytest_same_file_discovery_expected_output, + # ), ( "unittest_folder", expected_discovery_test_output.unittest_folder_discovery_expected_output, diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index b880b26fd5b8..d5d7d0e6a9f2 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -266,10 +266,10 @@ def build_test_tree(session: pytest.Session) -> TestNode: test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): try: - test_class_node = class_nodes_dict[test_case.parent.name] + test_class_node = class_nodes_dict[test_case.parent.nodeid] except KeyError: test_class_node = create_class_node(test_case.parent) - class_nodes_dict[test_case.parent.name] = test_class_node + class_nodes_dict[test_case.parent.nodeid] = test_class_node test_class_node["children"].append(test_node) if test_case.parent.parent: parent_module = test_case.parent.parent From 7107ce6dc4eaf57cff5364fb0e36c621bbf983d9 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 10 Jul 2023 14:42:03 -0700 Subject: [PATCH 2/2] uncomment tests --- .../tests/pytestadapter/test_discovery.py | 185 +++++++++--------- 1 file changed, 93 insertions(+), 92 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index d4bf7a32867a..02ea1ddcd871 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -9,103 +9,104 @@ from . import expected_discovery_test_output from .helpers import TEST_DATA_PATH, runner -# def test_import_error(tmp_path): -# """Test pytest discovery on a file that has a pytest marker but does not import pytest. - -# Copies the contents of a .txt file to a .py file in the temporary directory -# to then run pytest discovery on. - -# The json should still be returned but the errors list should be present. - -# Keyword arguments: -# tmp_path -- pytest fixture that creates a temporary directory. -# """ -# # Saving some files as .txt to avoid that file displaying a syntax error for -# # the extension as a whole. Instead, rename it before running this test -# # in order to test the error handling. -# file_path = TEST_DATA_PATH / "error_pytest_import.txt" -# temp_dir = tmp_path / "temp_data" -# temp_dir.mkdir() -# p = temp_dir / "error_pytest_import.py" -# shutil.copyfile(file_path, p) -# actual_list: Optional[List[Dict[str, Any]]] = runner( -# ["--collect-only", os.fspath(p)] -# ) -# assert actual_list -# for actual in actual_list: -# assert all(item in actual for item in ("status", "cwd", "error")) -# assert actual["status"] == "error" -# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) -# assert len(actual["error"]) == 2 - - -# def test_syntax_error(tmp_path): -# """Test pytest discovery on a file that has a syntax error. - -# Copies the contents of a .txt file to a .py file in the temporary directory -# to then run pytest discovery on. - -# The json should still be returned but the errors list should be present. - -# Keyword arguments: -# tmp_path -- pytest fixture that creates a temporary directory. -# """ -# # Saving some files as .txt to avoid that file displaying a syntax error for -# # the extension as a whole. Instead, rename it before running this test -# # in order to test the error handling. -# file_path = TEST_DATA_PATH / "error_syntax_discovery.txt" -# temp_dir = tmp_path / "temp_data" -# temp_dir.mkdir() -# p = temp_dir / "error_syntax_discovery.py" -# shutil.copyfile(file_path, p) -# actual = runner(["--collect-only", os.fspath(p)]) -# if actual: -# actual = actual[0] -# assert actual -# assert all(item in actual for item in ("status", "cwd", "error")) -# assert actual["status"] == "error" -# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) -# assert len(actual["error"]) == 2 - - -# def test_parameterized_error_collect(): -# """Tests pytest discovery on specific file that incorrectly uses parametrize. - -# The json should still be returned but the errors list should be present. -# """ -# file_path_str = "error_parametrize_discovery.py" -# actual = runner(["--collect-only", file_path_str]) -# if actual: -# actual = actual[0] -# assert all(item in actual for item in ("status", "cwd", "error")) -# assert actual["status"] == "error" -# assert actual["cwd"] == os.fspath(TEST_DATA_PATH) -# assert len(actual["error"]) == 2 + +def test_import_error(tmp_path): + """Test pytest discovery on a file that has a pytest marker but does not import pytest. + + Copies the contents of a .txt file to a .py file in the temporary directory + to then run pytest discovery on. + + The json should still be returned but the errors list should be present. + + Keyword arguments: + tmp_path -- pytest fixture that creates a temporary directory. + """ + # Saving some files as .txt to avoid that file displaying a syntax error for + # the extension as a whole. Instead, rename it before running this test + # in order to test the error handling. + file_path = TEST_DATA_PATH / "error_pytest_import.txt" + temp_dir = tmp_path / "temp_data" + temp_dir.mkdir() + p = temp_dir / "error_pytest_import.py" + shutil.copyfile(file_path, p) + actual_list: Optional[List[Dict[str, Any]]] = runner( + ["--collect-only", os.fspath(p)] + ) + assert actual_list + for actual in actual_list: + assert all(item in actual for item in ("status", "cwd", "error")) + assert actual["status"] == "error" + assert actual["cwd"] == os.fspath(TEST_DATA_PATH) + assert len(actual["error"]) == 2 + + +def test_syntax_error(tmp_path): + """Test pytest discovery on a file that has a syntax error. + + Copies the contents of a .txt file to a .py file in the temporary directory + to then run pytest discovery on. + + The json should still be returned but the errors list should be present. + + Keyword arguments: + tmp_path -- pytest fixture that creates a temporary directory. + """ + # Saving some files as .txt to avoid that file displaying a syntax error for + # the extension as a whole. Instead, rename it before running this test + # in order to test the error handling. + file_path = TEST_DATA_PATH / "error_syntax_discovery.txt" + temp_dir = tmp_path / "temp_data" + temp_dir.mkdir() + p = temp_dir / "error_syntax_discovery.py" + shutil.copyfile(file_path, p) + actual = runner(["--collect-only", os.fspath(p)]) + if actual: + actual = actual[0] + assert actual + assert all(item in actual for item in ("status", "cwd", "error")) + assert actual["status"] == "error" + assert actual["cwd"] == os.fspath(TEST_DATA_PATH) + assert len(actual["error"]) == 2 + + +def test_parameterized_error_collect(): + """Tests pytest discovery on specific file that incorrectly uses parametrize. + + The json should still be returned but the errors list should be present. + """ + file_path_str = "error_parametrize_discovery.py" + actual = runner(["--collect-only", file_path_str]) + if actual: + actual = actual[0] + assert all(item in actual for item in ("status", "cwd", "error")) + assert actual["status"] == "error" + assert actual["cwd"] == os.fspath(TEST_DATA_PATH) + assert len(actual["error"]) == 2 @pytest.mark.parametrize( "file, expected_const", [ - # ( - # "param_same_name", - # expected_discovery_test_output.param_same_name_expected_output, - # ), - # ( - # "parametrize_tests.py", - # expected_discovery_test_output.parametrize_tests_expected_output, - # ), - # ( - # "empty_discovery.py", - # expected_discovery_test_output.empty_discovery_pytest_expected_output, - # ), - # ( - # "simple_pytest.py", - # expected_discovery_test_output.simple_discovery_pytest_expected_output, - # ), - # ( - # "unittest_pytest_same_file.py", - # expected_discovery_test_output.unit_pytest_same_file_discovery_expected_output, - # ), + ( + "param_same_name", + expected_discovery_test_output.param_same_name_expected_output, + ), + ( + "parametrize_tests.py", + expected_discovery_test_output.parametrize_tests_expected_output, + ), + ( + "empty_discovery.py", + expected_discovery_test_output.empty_discovery_pytest_expected_output, + ), + ( + "simple_pytest.py", + expected_discovery_test_output.simple_discovery_pytest_expected_output, + ), + ( + "unittest_pytest_same_file.py", + expected_discovery_test_output.unit_pytest_same_file_discovery_expected_output, + ), ( "unittest_folder", expected_discovery_test_output.unittest_folder_discovery_expected_output,