Add tool version capture to Build Notes PDF#20
Conversation
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
- Use shortened job-IDs (e.g., int-win-8 instead of integration-windows-latest-dotnet8.x) to reduce report line length - Add step to display tool versions report to console after publishing - Update artifact path pattern to match new job-ID format Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot The integration test failed with: Why is it not able to find .versionmark.yaml as the file definitely exists in the root of the repo, and the build job seems to try to run the tool from the root as well? |
The integration-test job needs to checkout the repository to access the .versionmark.yaml configuration file required for version capture. Without this, the versionmark --capture command fails with "Configuration file not found". Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
... Fixed in 9025fe7. The integration-test job was missing a checkout step, so the .versionmark.yaml file wasn't present in the working directory. Added checkout step at the beginning of the job to ensure the configuration file is available for version capture. |
.github/workflows/build.yaml
Outdated
| OS_SHORT=$(echo "${{ matrix.os }}" | sed 's/windows-latest/win/;s/ubuntu-latest/ubuntu/') | ||
| DOTNET_SHORT=$(echo "${{ matrix.dotnet-version }}" | sed 's/\.x$//') | ||
| JOB_ID="int-${OS_SHORT}-${DOTNET_SHORT}" | ||
| versionmark --capture --job-id "${JOB_ID}" |
There was a problem hiding this comment.
This should only capture the tool versions used in the job - in this case probably just 'dotnet'. Trying to capture all tool versions will fail because not all tools are installed.
There was a problem hiding this comment.
Fixed in fe5c315. Now explicitly captures only dotnet and git (the tools actually available in the integration-test job).
.github/workflows/build.yaml
Outdated
| shell: bash | ||
| run: | | ||
| echo "Capturing tool versions..." | ||
| versionmark --capture --job-id "build-docs" |
There was a problem hiding this comment.
This may want to be explicit about which tools to capture version information for.
There was a problem hiding this comment.
Fixed in fe5c315. Now explicitly lists all tools to capture: dotnet, git, node, npm, and all dotnet tools (pandoc, weasyprint, sarifmark, sonarmark, reqstream, buildmark).
- integration-test job now captures only dotnet and git (tools actually available in that job) - build-docs job explicitly lists all tools to capture (dotnet, git, node, npm, and all dotnet tools) - Prevents failures from trying to capture tools that aren't installed Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot I got the following test failure: There seems to be something intermittent with this - can you evaluate? |
- Updated IntegrationTest_CaptureCommandWithDefaultOutput_UsesDefaultFilename to use temp directory with GUID - Updated IntegrationTest_CaptureCommand_CapturesToolVersions to use temp directory with GUID - Replaced all Path.GetRandomFileName() with GUID-based temp directories for consistency - Prevents race conditions when tests run in parallel by ensuring each test uses isolated temp directory - All tests now copy .versionmark.yaml to their own temp directory instead of sharing repo root Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
... Fixed in 04b6822. The intermittent failure was caused by a race condition - the test was creating files in the shared repository root directory without using unique names. Multiple parallel test executions could interfere with each other. Now all tests that write/delete files use isolated temp directories with GUIDs, preventing collisions during parallel execution. |
- Created TestDirectory disposable helper class for managing isolated test directories - Handles temp directory creation, optional config file copying, directory changes, and cleanup - Refactored IntegrationTest_CaptureCommand_CapturesToolVersions to use helper - Refactored IntegrationTest_CaptureCommandWithDefaultOutput_UsesDefaultFilename to use helper - Refactored IntegrationTest_CaptureCommandWithMissingConfig_ReturnsError to use helper - Refactored VersionMark_PublishCommand_GeneratesMarkdownReport to use helper - Moved FindRepositoryRoot inside TestDirectory class to avoid code duplication - Reduces boilerplate and improves test maintainability Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Malcolmnixon
left a comment
Malcolmnixon
left a comment
There was a problem hiding this comment.
You appear to have replace the PathHelpers.SafePathCombine with the Path.Combine. The Path.Combine is considered unsafe by CodeQL and produces warnings. Please make sure to use the save version.
… warnings - Updated TestDirectory constructor to use PathHelpers.SafePathCombine - Updated all temp directory creation in tests to use PathHelpers.SafePathCombine - Fixed IntegrationTest_CaptureCommand_CapturesToolVersions output file path - Ensures CodeQL security checks pass by using safe path combination method Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in 48c69df. Replaced all instances of |
|
@copilot I got the following error: I suspect it's because VersionMarkConfig.RunCommand doesn't have UseShellExecute=true, and I think As such I think the best approach is to modify the RunCommand to actually run "cmd.exe /c " or "/bin/sh -c " but use the ProcessStartInfo.ArgumentList approach to put the entire command in as a single string. For example: var isWin = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
var startInfo = new ProcessStartInfo
{
FileName = isWin ? "cmd.exe" : "/bin/sh",
UseShellExecute = false,
RedirectStandardOutput = true
};
startInfo.ArgumentList.Add(isWin ? "/c" : "-c");
startInfo.ArgumentList.Add(fullCommandLine); |
- Modified RunCommand to use cmd.exe /c on Windows and /bin/sh -c on Unix - Used ArgumentList instead of Arguments to avoid escaping issues - Check process exit code and throw exception for non-zero exit codes - This fixes npm.cmd and other batch file execution on Windows - Cross-platform solution that works without manual quote escaping Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in 2c77e51. Implemented your suggested approach using ArgumentList with cmd.exe /c on Windows and /bin/sh -c on Unix. This avoids escaping issues and properly supports .cmd and .bat files like npm on Windows. Added exit code checking to detect command failures. |
|
@copilot Please fix the following warning Then the markdown report needs to be modified for readability. The current formatting for different versions isn't readable. Instead Replicate multiple bullets (one for each version-group) rather than trying to put all versions in a single bullet. |
- Fixed inefficient ContainsKey warning in Validation.cs by using TryGetValue - Modified markdown formatter to use multiple bullets for different version groups - Removed "(All jobs)" text - uniform versions now show just the version - Removed subscript formatting - job IDs now shown in plain parentheses - Updated all tests to reflect new formatting - Improves readability when tools have different versions across jobs Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in 2eb14b4.
Example output: |
|
@copilot For the Version Report, let's drop the depth from 2 to 1 so it's a top-level section; and also in the Build Notes PDF, let's have it after the buildnotes.md |
- Changed --report-depth from 2 to 1 in build workflow to make Tool Versions a top-level section (#) - Reordered input-files in definition.yaml to place versions.md after buildnotes.md - This makes the version report a primary section in the Build Notes PDF and positions it after the main build notes content Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in 7a126f6. Changed |
Pull Request
Description
Build Notes PDF now includes versions of all tools used during build. Captures tool versions from integration-test matrix jobs (2 OSes × 3 .NET versions) and build-docs job, consolidates them, and publishes to
docs/buildnotes/versions.mdas a top-level section positioned after the main build notes content.Changes
.versionmark.yaml, captures tool versions (dotnet, git) post-validation with shortened job-IDs (e.g., "int-win-8" instead of "integration-windows-latest-dotnet8.x"), uploads JSON artifacts per matrix combinationdocs/buildnotes/definition.yamlas a top-level section (heading depth 1) positioned after buildnotes.md(job-1, job-2)) for clarity.TestDirectoryhelper class to reduce boilerplate and improve test maintainability. All path operations usePathHelpers.SafePathCombinefor CodeQL security compliance.Version consolidation handled by VersionMark's publish mode using glob patterns across all captured JSON files. Job-IDs are kept concise to prevent lengthy report lines. Tool lists are explicitly specified in capture commands to only capture tools actually available in each job context, preventing failures from attempting to capture unavailable tools.
Type of Change
Related Issues
Implements feature request for tool version visibility in build documentation.
Pre-Submission Checklist
Before submitting this pull request, ensure you have completed the following:
Build and Test
dotnet build --configuration Releasedotnet test --configuration Releasedotnet run --project src/DemaConsulting.VersionMark --configuration Release --framework net10.0--no-build -- --validateCode Quality
dotnet format --verify-no-changesQuality Checks
Please run the following checks before submitting:
cspell "**/*.{md,cs}"markdownlint "**/*.md"yamllint '**/*.{yml,yaml}'Testing
Documentation
Additional Notes
Workflow changes and production code modifications to support cross-platform command execution and improved markdown formatting. Local testing confirms capture/publish flow generates expected markdown output. CodeQL security scan passed with no vulnerabilities.
Job-IDs use shell script transformations to create concise identifiers (e.g., "int-win-8", "int-ubuntu-9") preventing lengthy report lines. The consolidated version report is displayed to the console for visibility during the build process.
The integration-test job includes a checkout step to ensure the
.versionmark.yamlconfiguration file is available for version capture operations. Both jobs explicitly specify which tools to capture: integration-test capturesdotnetandgit(the tools available in that context), while build-docs captures all available tools including dotnet tools restored viadotnet tool restore.Cross-Platform Command Execution: Modified
RunCommandinVersionMarkConfig.csto execute commands through the appropriate shell (cmd.exe /c on Windows, /bin/sh -c on Unix) usingArgumentListinstead ofArgumentsstring. This approach:Markdown Report Formatting: Updated
MarkdownFormatter.csto improve readability:18.0.0 (job-1, job-3)) instead of subscript formatting# Tool Versions) making it a primary section in the Build Notes PDFReport Positioning: The Tool Versions section is positioned after the main build notes content in the PDF through the
docs/buildnotes/definition.yamlfile ordering, providing a logical flow where build details are presented first, followed by the tool versions used to create the build.Code Quality Improvements: Fixed inefficient dictionary access in
Validation.csby replacing ContainsKey check followed by indexer access with a single TryGetValue call, eliminating duplicate dictionary lookups.Test Infrastructure Improvements: Addressed intermittent test failure caused by race conditions when tests ran in parallel. The issue was that tests were creating files in shared directories without unique names, causing conflicts. Solution:
versionmark-test-{Guid})TestDirectorydisposable helper class that encapsulates temp directory creation, optional config file copying, directory management, and automatic cleanupPathHelpers.SafePathCombineinstead ofPath.Combineto avoid CodeQL security warnings about unsafe path operationsOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.