Remote API should use paths for affected files relative to specified source directory

[akash-manna-sky]

Remote API should use paths for affected files relative to specified source directory

Fixes #3092

Testing done

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests that demonstrate the feature works or the issue is fixed

[akash-manna-sky]

@uhafner Please review the changes.

There is one PMD warning in IssuesPublisher.java‎ file at L306 regarding coupling within the class. I think this is not related my changes. Can you please help me how to fix this?

[uhafner]

Why do you check the "tests provided" task, when you do not test your code?

This is really essential: before changing code you must create a test case that exposes the problem!

[akash-manna-sky]

I added test cases for this change. Please review the changes. @uhafner

[uhafner]

This test actually does not test anything 🤔

When I remove your production code, the test already passes. This is a fundamental concept in software engineering: first write a test that fails. Then implement the changes.

[uhafner]

Can you please also have a look at FileNameResolver? There is also some code in the plugin and the model, that automatically creates relative paths in the issues. Maybe we need to pass the source folders to this method, then the model is correct automatically without any changes to the remote API.

[akash-manna-sky]

Can you please also have a look at FileNameResolver? There is also some code in the plugin and the model, that automatically creates relative paths in the issues. Maybe we need to pass the source folders to this method, then the model is correct automatically without any changes to the remote API.

The FileNameResolver.java belongs to analysis model and looked the implementation. I looked into the codebase, no FileNameResolve.java file found in this plugin. I think correct file to update is IssueScanner.java also suspect the AffectedFilesResolver.java file. I am getting confused about which files to update. Please review the current implementation. Can you suggest me which files are correct to update? please guide me with the initial approach to solve this issue? @uhafner

[uhafner]

Can you check, why the FileNameResolver does not already find the correct paths in the code above? Your code seems to do the things that the FileNameResolver already should do 🤔

[akash-manna-sky]

Yes, the FileNameResolver.java has a makeRelative method that calls PATH_UTIL.getRelativePath(sourceDirectoryPrefix, fileName) which already makes path relative to source directory. So basically makePathsRelativeToSourceDirectories method is duplicating what already FileNameResolver.java does. But the issue is that FileNameResolver only makes path relative if the file exists in that directory by the filter PATH_UTIL.exists(entry.getValue(), sourceDirectoryPrefix), which won't work for archive/historical build data. Here it is work even for archive data as here is no such filter.
So:
Input: X:/Archive/old-build/tasks/src/main/java/Foo.java
Source: X:/Archive/old-build/tasks/src
Output: main/java/Foo.java

What should I do now?

[uhafner]

But for historical build data the report is not changed at all, these reports are final.

So either the bug is not present anymore or we do oversee something. For me it does not make sense to create a duplicate of the method (where the only change is that the file might not exist anymore).

So if we cannot reproduce the issue in a real integration test (*ITest), I think I would probably close the issue as cannot reproduce.

I'm not sure if you want to spend some more time for an integration test that actually fails when we

• create a job in Jenkins
• create a source file within a sub folder
• create a fake warning for this source file
• run a build with the source prefix option
• check that the path is correctly mapped to the relative path
• call the remote API and see if the path is correctly mapped to the relative path

What do you think?

[akash-manna-sky]

I also suspect that the same because the issue was raised back in 2022 and I myself updated FileNameResolver.java file in analysis-model via PR jenkinsci/analysis-model#1279 to fix issue https://issues.jenkins.io/browse/JENKINS-75009. By which I think the issue is already resolved. So it will be better to add a dedicated integration test as well as unit test for the issue to show it is already resolved and close the issue. Other wise keep the PR open, I will reinvestigate it one more time.

[akash-manna-sky]

I have added an integration test in RemoteApiITest.java, and it passes without any production code changes. This confirms that the issue has already been resolved. So, it would be appropriate to close the issue, using these test cases as proof of the existing correct behavior. @uhafner

[akash-manna-sky]

Thank you!