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.

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

Improving baseliner #2290

Merged
merged 3 commits into from
Feb 24, 2021
Merged

Improving baseliner #2290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8c7260
Improving baseliner
eddynaka Feb 22, 2021
9aba407
Adding test to verify new behavior
eddynaka Feb 22, 2021
f454552
Merge branch 'main' into users/ednakamu/improving-baseliner
eddynaka Feb 24, 2021
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
8 changes: 8 additions & 0 deletions src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ public Result CalculateBasedlinedResult(DictionaryMergeBehavior propertyBagMerge
{
// Baseline result and current result have been matched => existing.
result = ConstructExistingResult(resultMatchingProperties, out originalResultMatchingProperties);

var previousResult = new Result { Locations = PreviousResult.Result.Locations };
var currenResult = new Result { Locations = CurrentResult.Result.Locations };

if (previousResult.ValueGetHashCode() != currenResult.ValueGetHashCode())
{
result.BaselineState = BaselineState.Updated;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure the location is accurate enough to perform this logic? The sarif files I've worked on generally change every time we make any changes at all to the files we're examining.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is our first logic to add the updated tag. so, it yet not precise, but it will work for our current case. probably this will not get merged since we found another way...

In reply to: 581470416 [](ancestors = 581470416)

}
}
else if (PreviousResult == null && CurrentResult != null)
{
Expand Down
24 changes: 23 additions & 1 deletion src/Test.UnitTests.Sarif/Baseline2/MatchedResultsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,29 @@ private struct FirstDetectionTimeTestCase
},
ExpectedBaselineState = BaselineState.Unchanged,
ExpectedFirstDetectionTime = previousRunEndTime
}
},
new FirstDetectionTimeTestCase
{
Name = "Updated, new location",
PreviousResult = new Result
{
Locations = new []
{
new Location { PhysicalLocation = new PhysicalLocation { ArtifactLocation = new ArtifactLocation{ Uri = new Uri("a.sarif", UriKind.Relative)}}}
}
},
PreviousRun = new Run(),
CurrentResult = new Result
{
Locations = new []
{
new Location { PhysicalLocation = new PhysicalLocation { ArtifactLocation = new ArtifactLocation{ Uri = new Uri("a.sarif", UriKind.Relative)}}},
new Location { PhysicalLocation = new PhysicalLocation { ArtifactLocation = new ArtifactLocation{ Uri = new Uri("b.sarif", UriKind.Relative)}}},
}
}, // Matches because the result matcher looks at only certain properties, and Provenance isn't one of them.
CurrentRun = new Run(),
ExpectedBaselineState = BaselineState.Updated,
},
};

[Fact]
Expand Down