Fix JSON parsing in tasks when '{' is in the GitVersion output#1792
Fix JSON parsing in tasks when '{' is in the GitVersion output#1792arturcic merged 3 commits intoGitTools:mainfrom
Conversation
|
@SimplyAName thanks for putting that much effort in this, I will hopefully have a look soon on the changes |
|
Awesome, thanks. The main code is working, just having some issues with my tests. |
|
All tests have been fixed now. Had to modify one of the existing tests that checks the SHA of the commit. Was failing due to the pipeline merging master in before running. This resulted in a new commit and hash being created, which did not match the branch being checked by GitVersion. Updated to get the latest commit without merge commits |
|
@SimplyAName please use rebase onto |
a938242 to
78402aa
Compare
|
@arturcic I've rebased my branch and force-pushed to override the merge commits. Lmk if anything needs fixing |
|
@SimplyAName please rebase your changes once again and fix the SonarCloud issue: I will accept the change for this release, but I think in the next major release we need to implement it in a more reliable way than parsing the output. There is and Anyway thanks for the time and effort. |
1498904 to
7fe7348
Compare
|
@SimplyAName I applied the changes myself and squashed the commits into 2 atomic commits |
e6808cd to
5568eea
Compare
42d5a4d to
94569c8
Compare
|
@arturcic Thanks for doing the fix! Yeah, that sounds best long term. I'll look into that in future, thanks :) I've rebased again after seeing some updates on the main branch. Branch commit: 94569c8 I've added a small update after the rebase to fix the commit hashes, but another rebase will cause it to fail again Can this be merged soon so Dependabot doesn't cause me to rebase again Thanks |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances GitVersion output parsing to handle edge cases where branch names contain curly braces ({ or }), which previously caused incorrect JSON extraction. The implementation introduces a new helper function and refactors the parsing logic to iteratively search for valid JSON within the output.
- Adds
allIndexesOf()utility function to find all occurrences of a character in a string - Refactors
processGitVersionOutput()to use a newextractGitVersionOutput()method with improved error handling - Adds comprehensive test coverage for the new JSON extraction logic, including edge cases with curly braces in branch names
Reviewed Changes
Copilot reviewed 4 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/lib.ts | Adds new allIndexesOf() utility function to find all character positions in a string |
| src/tools/gitversion/runner.ts | Refactors JSON extraction logic to handle curly braces in branch names, improves error handling |
| src/tests/tools/gitversion/runner.spec.ts | Adds test fixtures and test cases for new extraction logic, refactors commit hash retrieval |
| dist/tools/libs/gitversion.mjs | Compiled output reflecting runner.ts changes |
| dist/tools/lib.mjs | Compiled output reflecting lib.ts changes |
| dist/tools/libs/gitversion.mjs.map | Updated source map for gitversion.mjs |
| dist/tools/lib.mjs.map | Updated source map for lib.mjs |
| README.md | Improves badge labels and adds comprehensive contributing guidelines |
| .gitignore | Formatting improvements and adds test.env to ignored files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@SimplyAName please fix the small issues copilot found |
04a1c84 to
f6d4ee5
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f6d4ee5 to
31b3c2d
Compare
fixes the gitversion json parsing, including an extractor function to handle branch names with brackets adds a helper function to find all indexes of a character in a string adds tests for the json extractor to cover different branch names
31b3c2d to
75bc5fb
Compare
Uses simpleGit directly to retrieve the SHA hash. Removes custom function for getting the latest commit hash.
|
|
Thank you @SimplyAName for your contribution! |
I have not contributed here before, so please let me know if there's anything I need to do/fix.
I've done my best to match existing styling and have included some unit tests for my change.
TLDR:
This PR is mainly to fix #1711. I've been running into the same issue on my own pipelines and have tracked down the root cause to output parsing in the tasks.
The error occurs when a '{' character exists anywhere in the GitVersion output object, such as in a branch name. This caused the old parsing function to get an invalid JSON object, as the last '{' didn't belong to the JSON object, just the property string.
To fix this, I've added a function that will start with the old search (last '{' and '}') but then expand to the next '{' character and check if the JSON object is valid again. This continues till a valid object is found or sets the task to failed if it is unable to find one.
I've also included a new section on getting set up with this project in the README. I faced some issues setting it up for the first time, so I included some of the basic things I was missing. If the README changes should be in a separate PR, let me know