Add ability to test multiple OSes #275
Conversation
Switch build to any CPU
WalkthroughThe changes primarily focus on modifying the job configurations in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- azure-pipelines.yml (2 hunks)
Additional context used
yamllint
azure-pipelines.yml
[error] 24-24: trailing spaces (trailing-spaces)
[error] 40-40: wrong indentation: expected 2 but found 0 (indentation)
[error] 45-45: trailing spaces (trailing-spaces)
[error] 46-46: wrong indentation: expected 4 but found 2 (indentation)
[error] 64-64: line too long (82 > 80 characters) (line-length)
[error] 65-65: trailing spaces (trailing-spaces)
[error] 71-71: line too long (81 > 80 characters) (line-length)
[error] 72-72: trailing spaces (trailing-spaces)
[error] 74-74: line too long (119 > 80 characters) (line-length)
[error] 75-75: line too long (162 > 80 characters) (line-length)
[error] 76-76: trailing spaces (trailing-spaces)
[error] 80-80: line too long (98 > 80 characters) (line-length)
[error] 88-88: trailing spaces (trailing-spaces)
[error] 90-90: line too long (88 > 80 characters) (line-length)
[error] 91-91: trailing spaces (trailing-spaces)
[error] 101-101: line too long (87 > 80 characters) (line-length)
[error] 103-103: line too long (123 > 80 characters) (line-length)
[error] 115-115: trailing spaces (trailing-spaces)
[error] 122-122: trailing spaces (trailing-spaces)
[error] 139-139: line too long (103 > 80 characters) (line-length)
[error] 140-140: line too long (92 > 80 characters) (line-length)
[error] 141-141: line too long (87 > 80 characters) (line-length)
[error] 147-147: trailing spaces (trailing-spaces)
[error] 149-149: line too long (220 > 80 characters) (line-length)
[error] 153-153: trailing spaces (trailing-spaces)
[error] 172-172: line too long (83 > 80 characters) (line-length)
[error] 175-175: wrong indentation: expected 4 but found 2 (indentation)
[error] 191-191: trailing spaces (trailing-spaces)
[error] 193-193: trailing spaces (trailing-spaces)
[error] 199-199: line too long (94 > 80 characters) (line-length)
[error] 204-204: wrong indentation: expected 4 but found 2 (indentation)
[error] 242-242: line too long (98 > 80 characters) (line-length)
[error] 284-284: wrong indentation: expected 6 but found 8 (indentation)
[error] 286-286: line too long (104 > 80 characters) (line-length)
[error] 296-296: line too long (94 > 80 characters) (line-length)
[error] 308-308: wrong indentation: expected 6 but found 8 (indentation)
[error] 309-309: line too long (93 > 80 characters) (line-length)
[error] 341-341: line too long (104 > 80 characters) (line-length)
[error] 345-345: line too long (110 > 80 characters) (line-length)
[error] 375-375: trailing spaces (trailing-spaces)
[error] 388-388: trailing spaces (trailing-spaces)
[error] 425-425: line too long (98 > 80 characters) (line-length)
[error] 435-435: trailing spaces (trailing-spaces)
[error] 443-443: line too long (127 > 80 characters) (line-length)
[error] 467-467: line too long (86 > 80 characters) (line-length)
[error] 468-468: line too long (113 > 80 characters) (line-length)
[error] 469-469: line too long (97 > 80 characters) (line-length)
[error] 470-470: line too long (105 > 80 characters) (line-length)
[error] 471-471: line too long (98 > 80 characters) (line-length)
[error] 492-492: wrong indentation: expected 4 but found 2 (indentation)
[error] 505-505: wrong indentation: expected 4 but found 2 (indentation)
[error] 508-508: trailing spaces (trailing-spaces)
Additional comments not posted (1)
azure-pipelines.yml (1)
176-186: Dynamic VM image selection using$(imageName)enhances flexibility for multi-OS testing.Verification successful
The
imageNamevariable is correctly defined within the matrix strategy and is used in thevmImagesetting. There are no other occurrences ofimageNamethat need attention.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of `imageName` variable in the pipeline. # Test: Search for the definition of `imageName` and its usage. rg --type yaml $'imageName'Length of output: 243
Re-order tasks
Revert pipeline to x64
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
azure-pipelines.yml (1)
185-185: Consider adjusting the comment for clarity.The comment
# TODO: Attempt to aim for Arm64 buildscould be more actionable. Consider specifying what needs to be done or what is blocking the Arm64 builds.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
.github/workflows/smoketest-build.ymlis excluded by none and included by nonenanoFirmwareFlasher.Library/nanoFirmwareFlasher.Library.csprojis excluded by none and included by nonenanoFirmwareFlasher.Tool/nanoFirmwareFlasher.Tool.csprojis excluded by none and included by none
Files selected for processing (3)
- azure-pipelines.yml (1 hunks)
- nanoFirmwareFlasher.Library/packages.lock.json (2 hunks)
- nanoFirmwareFlasher.Tool/packages.lock.json (1 hunks)
Files skipped from review due to trivial changes (2)
- nanoFirmwareFlasher.Library/packages.lock.json
- nanoFirmwareFlasher.Tool/packages.lock.json
Additional context used
yamllint
azure-pipelines.yml
[error] 24-24: trailing spaces (trailing-spaces)
[error] 40-40: wrong indentation: expected 2 but found 0 (indentation)
[error] 45-45: trailing spaces (trailing-spaces)
[error] 46-46: wrong indentation: expected 4 but found 2 (indentation)
[error] 64-64: line too long (82 > 80 characters) (line-length)
[error] 65-65: trailing spaces (trailing-spaces)
[error] 71-71: line too long (81 > 80 characters) (line-length)
[error] 72-72: trailing spaces (trailing-spaces)
[error] 74-74: line too long (119 > 80 characters) (line-length)
[error] 75-75: line too long (162 > 80 characters) (line-length)
[error] 76-76: trailing spaces (trailing-spaces)
[error] 80-80: line too long (98 > 80 characters) (line-length)
[error] 88-88: trailing spaces (trailing-spaces)
[error] 90-90: line too long (88 > 80 characters) (line-length)
[error] 91-91: trailing spaces (trailing-spaces)
[error] 101-101: line too long (87 > 80 characters) (line-length)
[error] 103-103: line too long (123 > 80 characters) (line-length)
[error] 115-115: trailing spaces (trailing-spaces)
[error] 122-122: trailing spaces (trailing-spaces)
[error] 139-139: line too long (103 > 80 characters) (line-length)
[error] 140-140: line too long (92 > 80 characters) (line-length)
[error] 141-141: line too long (87 > 80 characters) (line-length)
[error] 147-147: trailing spaces (trailing-spaces)
[error] 149-149: line too long (220 > 80 characters) (line-length)
[error] 153-153: trailing spaces (trailing-spaces)
[error] 172-172: line too long (83 > 80 characters) (line-length)
[error] 175-175: wrong indentation: expected 4 but found 2 (indentation)
[error] 183-183: trailing spaces (trailing-spaces)
[error] 191-191: line too long (94 > 80 characters) (line-length)
[error] 196-196: wrong indentation: expected 4 but found 2 (indentation)
[error] 216-216: trailing spaces (trailing-spaces)
[error] 239-239: line too long (98 > 80 characters) (line-length)
[error] 281-281: wrong indentation: expected 6 but found 8 (indentation)
[error] 283-283: line too long (104 > 80 characters) (line-length)
[error] 293-293: line too long (94 > 80 characters) (line-length)
[error] 305-305: wrong indentation: expected 6 but found 8 (indentation)
[error] 306-306: line too long (93 > 80 characters) (line-length)
[error] 338-338: line too long (104 > 80 characters) (line-length)
[error] 342-342: line too long (110 > 80 characters) (line-length)
[error] 376-376: trailing spaces (trailing-spaces)
[error] 389-389: trailing spaces (trailing-spaces)
[error] 426-426: line too long (98 > 80 characters) (line-length)
[error] 436-436: trailing spaces (trailing-spaces)
[error] 444-444: line too long (127 > 80 characters) (line-length)
[error] 469-469: line too long (86 > 80 characters) (line-length)
[error] 470-470: line too long (113 > 80 characters) (line-length)
[error] 471-471: line too long (97 > 80 characters) (line-length)
[error] 472-472: line too long (105 > 80 characters) (line-length)
[error] 473-473: line too long (98 > 80 characters) (line-length)
[error] 495-495: wrong indentation: expected 4 but found 2 (indentation)
[error] 508-508: wrong indentation: expected 4 but found 2 (indentation)
[error] 511-511: trailing spaces (trailing-spaces)
Additional comments not posted (2)
azure-pipelines.yml (2)
185-185: Review all jobs and steps for potential issues related to the change in the VM image.Verification successful
We need to verify if there are any specific configurations or steps in the
azure-pipelines.ymlfile that depend on thewindows-latestimage or x64 architecture. This will help us identify any potential issues when switching to Arm64 builds.
The search results indicate that the only reference to
x64in theazure-pipelines.ymlfile is the specific line you mentioned:value: 'x64' # TODO: Attempt to aim for Arm64 buildsThis suggests that there are no other configurations or steps explicitly tied to the x64 architecture, and the comment is a reminder to attempt Arm64 builds in the future.
- : The code does not show removed or replaced code, and the comment about aiming for Arm64 builds is accurate. No other steps or configurations in the pipeline are specifically tied to the x64 architecture.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all jobs and steps in the azure-pipelines.yml file are compatible with the new VM image settings. # Test: Search for any job or step configurations that might be specific to the old VM image. rg --type yaml $'windows-latest'Length of output: 181
Script:
#!/bin/bash # Description: Verify if there are any steps or configurations specific to the x64 architecture in the azure-pipelines.yml file. # Test: Search for any job or step configurations that mention 'x64'. rg --type yaml 'x64' azure-pipelines.ymlLength of output: 100
185-185: Ensure the$(imageName)variable is defined and correctly configured in the pipeline settings.
| @@ -182,7 +182,7 @@ jobs: | |||
| - name: DOTNET_NOLOGO | |||
| value: true | |||
| - name: buildPlatform | |||
| value: 'x64' | |||
| value: 'x64' # TODO: Attempt to aim for Arm64 builds | |||
There was a problem hiding this comment.
Update Windows-specific elements for compatibility with Ubuntu.
Please ensure that all Windows-specific elements such as PowerShell scripts, Windows-style paths, and environment variables are updated or replaced to ensure compatibility with the ubuntu-latest VM image.
|
|
||
| strategy: | ||
| matrix: | ||
| os: [windows-latest, ubuntu-latest, macos-latest] |
There was a problem hiding this comment.
I'm not a super huge fan of testing on MacOS for costs reasons. So we will have to be very careful or run it on demand.
There was a problem hiding this comment.
The macos-latest is a hosted (free) runner, To get the "full" benefit, it would be great to run on the macos-latest-xlarge image (as it builds on arm64) but that does indeed incur costs. Ultimately we need to wait for a free linux runner that is able to run on arm64.
There was a problem hiding this comment.
But, would also add that there are lots of warnings that are made visible by having this smoke test.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
nanoFirmwareFlasher.Tool/nanoFirmwareFlasher.Tool.csprojis excluded by none and included by none
Files selected for processing (1)
- azure-pipelines.yml (1 hunks)
Additional context used
yamllint
azure-pipelines.yml
[error] 24-24: trailing spaces (trailing-spaces)
[warning] 40-40: wrong indentation: expected 2 but found 0 (indentation)
[error] 45-45: trailing spaces (trailing-spaces)
[warning] 46-46: wrong indentation: expected 4 but found 2 (indentation)
[error] 65-65: trailing spaces (trailing-spaces)
[error] 71-71: trailing spaces (trailing-spaces)
[error] 73-73: trailing spaces (trailing-spaces)
[error] 97-97: trailing spaces (trailing-spaces)
[error] 104-104: trailing spaces (trailing-spaces)
[error] 129-129: trailing spaces (trailing-spaces)
[error] 135-135: trailing spaces (trailing-spaces)
[warning] 157-157: wrong indentation: expected 4 but found 2 (indentation)
[error] 165-165: trailing spaces (trailing-spaces)
[warning] 176-176: wrong indentation: expected 4 but found 2 (indentation)
[error] 196-196: trailing spaces (trailing-spaces)
[warning] 261-261: wrong indentation: expected 6 but found 8 (indentation)
[warning] 285-285: wrong indentation: expected 6 but found 8 (indentation)
[error] 356-356: trailing spaces (trailing-spaces)
[error] 369-369: trailing spaces (trailing-spaces)
[error] 416-416: trailing spaces (trailing-spaces)
[warning] 460-460: wrong indentation: expected 4 but found 2 (indentation)
[warning] 473-473: wrong indentation: expected 4 but found 2 (indentation)
[error] 476-476: trailing spaces (trailing-spaces)
Additional comments not posted (4)
azure-pipelines.yml (4)
167-167: Address formatting issues and trailing spaces.There are several formatting issues and trailing spaces throughout the file as indicated by the static analysis tools. These should be cleaned up to maintain code quality and readability.
- value: 'x64' # TODO: Attempt to aim for Arm64 builds + value: 'x64' # TODO: Attempt to aim for Arm64 buildsLikely invalid or redundant comment.
167-167: Ensure compatibility of scripts with multiple OSes.The pipeline uses several PowerShell scripts. With the introduction of 'ubuntu-latest' as a VM image, ensure these scripts are compatible with Linux environments or provide equivalent Bash scripts.
167-167: UpdatevmImageto use variable-based settings.Changing
vmImagefrom a hardcoded 'windows-latest' to a variable-based setting ($(imageName)) is a good practice for flexibility. However, ensure that the variableimageNameis properly defined and managed within the pipeline to avoid configuration errors.
167-167: Consider the implications of the TODO comment for Arm64 builds.The TODO comment suggests an aim to support Arm64 builds. This is a significant architectural shift that might require additional changes beyond just the build platform setting. Ensure that the entire pipeline, including dependencies and scripts, supports Arm64 architecture.
Description
Adds smoke tests for compiling the tool against multiple OSes and framework versions.
Motivation and Context
Improves ability to ensure the tool will work when used against them.
How Has This Been Tested?
Screenshots
Types of changes
Checklist:
Summary by CodeRabbit
vmImagesettings in pipeline configuration for enhanced flexibility.Nerdbank.GitVersioningpackage to version3.6.139for improved performance and security.