Skip to content

Fixing of finding logic for XML paths#454

Open
MaximilianSoerenPollak wants to merge 1 commit intoeclipse-score:mainfrom
MaximilianSoerenPollak:MSP_quickfix_testlinks
Open

Fixing of finding logic for XML paths#454
MaximilianSoerenPollak wants to merge 1 commit intoeclipse-score:mainfrom
MaximilianSoerenPollak:MSP_quickfix_testlinks

Conversation

@MaximilianSoerenPollak
Copy link
Contributor

📌 Description

🚨 Impact Analysis

  • This change does not violate any tool requirements and is covered by existing tool requirements
  • This change does not violate any design decisions
  • Otherwise I have created a ticket for new tool qualification

✅ Checklist

  • Added/updated documentation for new or changed features
  • Added/updated tests to cover the changes
  • Followed project coding standards and guidelines

@github-actions
Copy link

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //src:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 1e7e8a0a-3108-4daa-8f6f-d7e71361d280
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: src
Loading: 0 packages loaded
    currently loading: src
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //src:license-check (59 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (70 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (76 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (128 packages loaded, 1521 targets configured)

Analyzing: target //src:license-check (138 packages loaded, 2552 targets configured)

Analyzing: target //src:license-check (140 packages loaded, 2576 targets configured)

Analyzing: target //src:license-check (140 packages loaded, 2576 targets configured)

Analyzing: target //src:license-check (140 packages loaded, 2576 targets configured)

Analyzing: target //src:license-check (143 packages loaded, 4465 targets configured)

Analyzing: target //src:license-check (143 packages loaded, 4465 targets configured)

Analyzing: target //src:license-check (143 packages loaded, 4465 targets configured)

Analyzing: target //src:license-check (143 packages loaded, 4465 targets configured)

INFO: Analyzed target //src:license-check (145 packages loaded, 4715 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions, 1 running)
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
INFO: Found 1 target...
Target //src:license.check.license_check up-to-date:
  bazel-bin/src/license.check.license_check
  bazel-bin/src/license.check.license_check.jar
INFO: Elapsed time: 24.411s, Critical Path: 2.59s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/src/license.check.license_check src/formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions
Copy link

The created documentation from the pull request is available at: docu-html

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

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

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants