Skip to content

Extensive code and testing cleanup#59

Merged
Malcolmnixon merged 2 commits intomainfrom
code-cleanup
Jul 6, 2025
Merged

Extensive code and testing cleanup#59
Malcolmnixon merged 2 commits intomainfrom
code-cleanup

Conversation

@Malcolmnixon
Copy link
Copy Markdown
Member

@Malcolmnixon Malcolmnixon commented Jul 6, 2025

This PR performs extensive code cleanup and restructuring, including:

  • Solution-wide code cleanup of all files.
  • Apply the detailed arrange/act/assert structure to all tests with supporting documentation
  • Update project dependencies

Apply the detailed arrange/act/assert structure to all tests.
@Malcolmnixon Malcolmnixon requested a review from Copilot July 6, 2025 02:06

This comment was marked as outdated.

@Malcolmnixon Malcolmnixon requested a review from Copilot July 6, 2025 02:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR restructures and cleans up code across the solution, refactors all tests to a consistent Arrange–Act–Assert style (with improved naming and XML doc comments), and bumps several project and tool dependencies.

  • Standardize test method names and add descriptive comments
  • Apply new C# 12 collection/array expression syntax and clone patterns in deep-copy implementations
  • Upgrade NuGet and .NET tool versions, including System.Text.Json, MSTest, and sbom-tool dependencies

Reviewed Changes

Copilot reviewed 61 out of 68 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/DemaConsulting.SpdxModel/Transform/SpdxRelationships.cs Add line wrapping and C#12 collection expressions
test/** Rename tests, apply Arrange/Act/Assert sections, update XML docs
*.csproj & dotnet-tools.json Bump dependency versions
src/DemaConsulting.SpdxModel/Spdx* & IO/*.cs Implement deep-copy braces, spread operators, Enhance refactors
DemaConsulting.SpdxModel.sln.DotSettings Add a ReSharper formatting setting
Comments suppressed due to low confidence (1)

DemaConsulting.SpdxModel.sln.DotSettings:1

  • The XML namespace URI is misspelled as shemas-jetbrains-com. It should be schemas-jetbrains-com.
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">

@@ -43,7 +45,8 @@ public static void Add(SpdxDocument document, SpdxRelationship relationship)

// Ensure the relationship related-element ID matches an element
if (document.GetElement(relationship.RelatedSpdxElement) == null)
Copy link

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

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

Add a check to validate that the Id of the incoming relationship also exists in document before adding. Currently only RelatedSpdxElement is validated, so bad Id values slip through without throwing.

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +49
throw new ArgumentException($"Element {relationship.RelatedSpdxElement} not found in SPDX document",
nameof(relationship));
Copy link

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

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

The thrown ArgumentException uses the two-parameter ctor which prepends a new line and Parameter name: text. Tests expect a single-line message with (Parameter 'relationship'). Either adjust the exception message formatting or update tests to assert against ex.ParamName and the actual multi-line Message.

Suggested change
throw new ArgumentException($"Element {relationship.RelatedSpdxElement} not found in SPDX document",
nameof(relationship));
throw new ArgumentException($"Element {relationship.RelatedSpdxElement} not found in SPDX document (Parameter '{nameof(relationship)}')");

Copilot uses AI. Check for mistakes.
@Malcolmnixon Malcolmnixon merged commit 89e1221 into main Jul 6, 2025
7 checks passed
@Malcolmnixon Malcolmnixon deleted the code-cleanup branch July 6, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants