Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR applies various code and security cleanups by improving test assertions, standardizing exception messages in SPDX model classes, and tightening permissions in GitHub workflow configurations.
- Updated tests to use StringAssert.StartsWith and improved error messages.
- Standardized exception message formatting in SPDX model classes.
- Added explicit permissions for workflow jobs to enhance security.
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SpdxModel.Tests/Transforms/SpdxRelationshipsTests.cs | Updated exception assertion to include an explicit failure message. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize23.cs | Renamed test method and replaced Assert.IsTrue with StringAssert.StartsWith; corrected a contributor string. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize22.cs | Renamed test method, replaced Assert.IsTrue with StringAssert.StartsWith, and fixed string literal typos. |
| src/DemaConsulting.SpdxModel/SpdxRelationshipType.cs | Standardized exception message formatting by adding quotes around the unsupported parameter. |
| src/DemaConsulting.SpdxModel/SpdxReferenceCategory.cs | Standardized exception message formatting by adding quotes around the unsupported parameter. |
| src/DemaConsulting.SpdxModel/SpdxFileType.cs | Standardized exception message formatting by adding quotes around the unsupported parameter. |
| src/DemaConsulting.SpdxModel/SpdxChecksumAlgorithm.cs | Standardized exception message formatting by adding quotes around the unsupported parameter. |
| .github/workflows/release.yaml | Added permissions block to the release workflow. |
| .github/workflows/build_on_push.yaml | Added permissions blocks to both Windows and Linux build jobs. |
| .github/workflows/build.yaml | Added permissions block to the build job configuration. |
Files not reviewed (2)
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.2.spdx.json: Language not supported
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.3.spdx.json: Language not supported
Added tests for code-coverage.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements code and security cleanups by improving test assertions, standardizing error messages and text conversions, and updating workflow permissions for enhanced security.
- Improved test validations using StringAssert and renamed test methods for clarity.
- Updated extension methods to handle case-insensitive input with ToUpperInvariant() and refined error messages.
- Enhanced GitHub workflow configurations by adding explicit permission settings.
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SpdxModel.Tests/Transforms/SpdxRelationshipsTests.cs | Improved exception assertion messaging in tests. |
| test/DemaConsulting.SpdxModel.Tests/SpdxRelationshipTests.cs | Added tests for relationship type conversions. |
| test/DemaConsulting.SpdxModel.Tests/SpdxFileTests.cs | Introduced tests for file type conversion methods. |
| test/DemaConsulting.SpdxModel.Tests/SpdxChecksumTests.cs | Added tests for checksum algorithm conversion methods. |
| test/DemaConsulting.SpdxModel.Tests/SpdxAnnotationTests.cs | Expanded tests for annotation type conversion methods. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize23.cs | Renamed test method and improved assertion methods for JSON deserialization. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize22.cs | Renamed test method and corrected spelling in assertions. |
| src/DemaConsulting.SpdxModel/*.cs | Standardized text conversion methods with case-insensitive handling and refined error messages. |
| .github/workflows/*.yaml | Updated workflow files with explicit permission settings for better security. |
Files not reviewed (2)
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.2.spdx.json: Language not supported
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.3.spdx.json: Language not supported
Comments suppressed due to low confidence (2)
src/DemaConsulting.SpdxModel/SpdxRelationshipType.cs:272
- [nitpick] Using ToUpperInvariant() ensures case-insensitive matching; verify that this approach aligns with overall design requirements and does not affect scenarios expecting case sensitivity.
return relationshipType.ToUpperInvariant() switch
.github/workflows/release.yaml:24
- The explicit permission settings (contents: read and pull-requests: write) enhance security; ensure these settings cover all necessary scopes for your CI pipeline without overprivileging.
permissions:
test/DemaConsulting.SpdxModel.Tests/Transforms/SpdxRelationshipsTests.cs
Outdated
Show resolved
Hide resolved
…psTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR focuses on code and security cleanups by standardizing exception message formatting in various SPDX-related extension methods, expanding test coverage for SPDX type conversions, and enhancing GitHub workflow security through explicit permission settings.
- Updated extension methods to use ToUpperInvariant and enclose input values in quotes within exception messages.
- Added comprehensive tests for FromText/ToText conversion methods in SPDX relationships, file types, checksum algorithms, and annotations.
- Enhanced GitHub workflow files by introducing explicit permission configurations.
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/DemaConsulting.SpdxModel.Tests/* | Expanded test coverage with improved assertions and validation of exception messages. |
| src/DemaConsulting.SpdxModel/* | Updated extension methods for SPDX models with standardized exception messages via ToUpperInvariant. |
| .github/workflows/release.yaml, build_on_push.yaml, build.yaml | Added explicit permission blocks to improve security and clarity in CI/CD workflows. |
Files not reviewed (2)
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.2.spdx.json: Language not supported
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.3.spdx.json: Language not supported
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR applies code and security cleanups throughout the project. The changes standardize error messages to include quotes, update switch expressions to use invariant casing, add more tests for SPDX extension methods, and adjust GitHub workflow permissions for release and build jobs.
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SpdxModel.Tests/Transforms/SpdxRelationshipsTests.cs | Adjusts exception capture and asserts expected error message. |
| test/DemaConsulting.SpdxModel.Tests/SpdxRelationshipTests.cs | Adds tests for relationship type conversion methods with varied casing. |
| test/DemaConsulting.SpdxModel.Tests/SpdxFileTests.cs | Updates tests with refined expected strings and corrects a contributor string typo. |
| test/DemaConsulting.SpdxModel.Tests/SpdxChecksumTests.cs | Introduces tests for checksum algorithm conversions. |
| test/DemaConsulting.SpdxModel.Tests/SpdxAnnotationTests.cs | Adds tests for annotation type conversion methods and uses StringAssert for clarity. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize23.cs | Renames test method and refines string assertions for better clarity. |
| test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserialize22.cs | Renames test method and refines string assertions for consistency. |
| src/DemaConsulting.SpdxModel/*.cs | Updates SPDX extensions to use ToUpperInvariant() and improves error message formatting. |
| .github/workflows/*.yaml | Adds explicit permissions to workflows to support new release and build requirements. |
Files not reviewed (2)
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.2.spdx.json: Language not supported
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.3.spdx.json: Language not supported
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR applies a series of code and security cleanups across the test suites, source code, and GitHub workflow configuration to improve consistency, error messaging, and permissions.
- Updates test assertions for better string matching and corrected typos in sample data.
- Refactors extension methods to normalize text inputs using ToUpperInvariant and standardizes exception messages with quotes.
- Adds/revises permissions in GitHub workflows to align with security best practices.
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/DemaConsulting.SpdxModel.Tests/* | Updates to test assertions and method renaming for clarity and consistency |
| src/DemaConsulting.SpdxModel/* | Refactorings in extension methods for input normalization and standardized exception messages |
| .github/workflows/*.yaml | Updates to workflow files adding explicit permissions to enhance security |
Files not reviewed (2)
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.2.spdx.json: Language not supported
- test/DemaConsulting.SpdxModel.Tests/IO/Examples/SPDXJSONExample-v2.3.spdx.json: Language not supported
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR applies numerous code and security cleanups.