Skip to content
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.

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

Update .NET ITs results to contain the GitHub URL instead of relative path #7548

Merged
merged 7 commits into from
Jul 13, 2023

Conversation

zsolt-kolbay-sonarsource
Copy link
Contributor

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource commented Jul 6, 2023

Changes the uri from a relative local path to a clickable url pointing to Github with line numbers.
e.g.
from "sources\Automapper\src\AutoMapper\Execution\TypeMapPlanBuilder.cs"
to "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Automapper/src/AutoMapper/Execution/TypeMapPlanBuilder.cs#L401"

If an issue location spans multiple lines then that range is added to the end of the url: #L123-126

Review:

  • the two Powershell scripts that were updated
  • randomly check the json files and look for issues that are in a single line, as well as issues, where a location spans multiple lines, check whether the links are valid

Note: The order of the issues can change in a JSON file if one or more of the issue's locations is the Temporary-Folder, because these remain unchanged, while all the other locations are converted to urls.

@andrei-epure-sonarsource
Copy link
Contributor

andrei-epure-sonarsource commented Jul 6, 2023

The GH UI is very slow, I am moving to PR-level comments rather than file comments.

index could be renamed to subPathStart (IMO more readable)

image

@andrei-epure-sonarsource
Copy link
Contributor

regression-test.ps1 - line 337-338

# checks if one file path ends without another without their line number suffix (Program.cs#12 and Program.cs#34 -> true)
function EndsWithLineNumbersRemoved([string]$path1, [string]$path2) {

IMO the comment and name are cumbersome to understand.

Rather than saying what the function does from a mechanical POV (we can read the code for that...) - what is the functionality of this function?

Suggestion: IsSameFile

Also, I'd change the comment to something:

# checks if the file in `path2` is the same as the file in `path1` 
# example:
# path1: a/b/Program.cs#12
# path2: b/Program.cs#34
# result: true

@andrei-epure-sonarsource
Copy link
Contributor

create-issue-reports.ps1 - Restore-UriDeclaration

In Restore-UriDeclaration, you removed $_.uri = $_.uri -replace "file:///" because now you're creating the URL.

So the # Remove the URI prefix comment seems to be out of date now.

Copy link
Contributor

@andrei-epure-sonarsource andrei-epure-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM!

I verified (sampling) the 68699 modified URLs, out of which 329 are multi-line.

I believe it's good to have the multi-line URLs feature, as it gives a rough impression of the highlighting (even if it's not the fully accurate information, as it's missing columns).

@andrei-epure-sonarsource
Copy link
Contributor

@zsolt-kolbay-sonarsource please give a heads-up to the whole squad when merging this, because everyone will need to rebase their PRs that have changed IT results

@andrei-epure-sonarsource andrei-epure-sonarsource changed the title Integration Test Improvements Update .NET ITs results to contain the github URL Jul 13, 2023
@andrei-epure-sonarsource andrei-epure-sonarsource changed the title Update .NET ITs results to contain the github URL Update .NET ITs results to contain the GitHub URL instead of relative path Jul 13, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource marked this pull request as ready for review July 13, 2023 09:29
@sonarcloud
Copy link

sonarcloud bot commented Jul 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jul 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource merged commit 17192a3 into master Jul 13, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource deleted the Zsolt/IT-improvements branch July 13, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Tooling Tools make us productive.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants