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

Jump to bottom

HdfConverter improvements #2699

Merged
merged 5 commits into from
Jul 27, 2023
Merged

HdfConverter improvements #2699

merged 5 commits into from
Jul 27, 2023

Conversation

candrews
Copy link
Collaborator

Various improvements to HdfConverter, fixing GitHub validation errors, adding new SARIF support, and improving SARIF compliance.

@candrews candrews force-pushed the hdfconverter-improvements branch 2 times, most recently from ed04f1e to 2e15243 Compare July 24, 2023 21:48
@michaelcfanning
Copy link
Member

You'll need to fix up the input files, e.g., at src\Test.UnitTests.Sarif.Converters\TestData\HdfConverter\Inputs\ValidResults.json to adhere to the new output format.

2023-07-24T21:40:10.1310592Z [xUnit.net 00:00:02.20] Microsoft.CodeAnalysis.Sarif.Converters.HdfConverterTests.Converter_WhenInputIsEmpty_ReturnsNoResults [FAIL]
2023-07-24T21:40:10.5082924Z [xUnit.net 00:00:02.52] Microsoft.CodeAnalysis.Sarif.Converters.HdfConverterTests.Converter_WhenInputContainsValidResults_ReturnsExpectedOutput [FAIL]

@candrews candrews force-pushed the hdfconverter-improvements branch from 2e15243 to 57ad0de Compare July 25, 2023 01:37
candrews added 5 commits July 24, 2023 21:45
@candrews
HdfConverter: Add location, address GitHub validation error (#2694)
6688dd8 
Per https://sarifweb.azurewebsites.net/Validation
> Each result location must provide the property 'physicalLocation.artifactLocation.uri'.
> GitHub Advanced Security code scanning will not display a result whose location does not provide the URI of the artifact that contains the result.

Signed-off-by: Craig Andrews <[email protected]>
@candrews
HdfConvert: Use HDF "control.descriptions" for "help" (#2633)
dfa8ec1 
Signed-off-by: Craig Andrews <[email protected]>
@candrews
HdfConverter: Use HDF title as SARIF shortDescription
93ef5ce
@candrews
HdfConverter: make name a Pascal case identifier
5e35f45 
See: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317843
Signed-off-by: Craig Andrews <[email protected]>
@candrews
HdfConverter: Set Enabled to true if and only if all results have sta…
4ff1394 
…tus skipped

See: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317854
Signed-off-by: Craig Andrews <[email protected]>
@candrews candrews force-pushed the hdfconverter-improvements branch from 57ad0de to 4ff1394 Compare July 25, 2023 01:45
@candrews
Copy link
Collaborator Author

You'll need to fix up the input files, e.g., at src\Test.UnitTests.Sarif.Converters\TestData\HdfConverter\Inputs\ValidResults.json to adhere to the new output format.

All set! The tests are all now passing.

@candrews
Copy link
Collaborator Author

@michaelcfanning could you please merge this PR?

@michaelcfanning michaelcfanning merged commit d9b7272 into main Jul 27, 2023
@michaelcfanning michaelcfanning deleted the hdfconverter-improvements branch July 27, 2023 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelcfanning michaelcfanning Awaiting requested review from michaelcfanning michaelcfanning is a code owner

@EasyRhinoMSFT EasyRhinoMSFT Awaiting requested review from EasyRhinoMSFT EasyRhinoMSFT is a code owner

@marmegh marmegh Awaiting requested review from marmegh

@cfaucon cfaucon Awaiting requested review from cfaucon cfaucon is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@candrews @michaelcfanning