Skip to content

Fixing of finding logic for XML paths #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MaximilianSoerenPollak wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −9
Open

Fixing of finding logic for XML paths #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/extensions/score_source_code_linker/tests/test_xml_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def test_find_xml_files_test_reports(
dir1: Path
dir2: Path
root, dir1, dir2, dir3, dir4 = tmp_xml_dirs(test_folder="tests-report")
found = xml_parser.find_xml_files(dir=root)
found = xml_parser.find_xml_files(search_path=root)
assert found is not None
expected: set[Path] = {
root / dir1 / "test.xml",
Expand Down
10 changes: 2 additions & 8 deletions src/extensions/score_source_code_linker/xml_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def read_test_xml_file(file: Path) -> tuple[list[DataOfTestCase], list[str], lis
return test_case_needs, non_prop_tests, missing_prop_tests


def find_xml_files(dir: Path) -> list[Path]:
def find_xml_files(search_path: Path) -> list[Path]:
"""
Recursively search all test.xml files inside 'bazel-testlogs'

Expand All @@ -260,13 +260,7 @@ def find_xml_files(dir: Path) -> list[Path]:
"""

test_file_name = "test.xml"

xml_paths: list[Path] = []
for root, _, files in os.walk(dir):
if test_file_name in files:
xml_paths.append(Path(os.path.join(root, test_file_name)))

return xml_paths
return [x for x in search_path.rglob(test_file_name)]
Copy link
Contributor

@a-zw a-zw Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this change behavior?

Coincidentally, I know that rglob does not follow symlinks while os.walk does. At least, not by default and not with our Python 3.12. Is that the relevant change here?

Copy link
Contributor

@PiotrKorkus PiotrKorkus Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

[x.resolve() for x in search_path.rglob(test_file_name)]

will follow symlinks

Copy link
Contributor

@a-zw a-zw Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so that is not the change. What is it? From looking at it the code looks equivalent. 🤔

Copy link
Contributor Author

@MaximilianSoerenPollak MaximilianSoerenPollak Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is equivalent, but the os.walk did behave differently than this code does.

This finds all of the files locally & in ref-integration as well as ci/cd

THe old code did work locally but in CI/CD in ref-integration it didn't work anymore and found 0 test files.
Even though the path was correct (that it searched) and the test files were there.

Copy link
Contributor Author

@MaximilianSoerenPollak MaximilianSoerenPollak Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change here:

for root, dirs, files in os.walk(dir, topdown=False, followlinks=True):

Also worked, though I found the now proposed solution cleaner to read



def find_test_folder(base_path: Path | None = None) -> Path | None:
Expand Down
Loading