Improve assembly dump #178
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (7)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe change introduces a new task sequence in the Azure Pipelines YAML file. It adds tasks that first copy Visual Studio test dump files from a temporary location to the artifact staging directory. A PowerShell script then checks for the existence of these files and sets a variable accordingly. Finally, a publishing task conditionally runs based on whether dump files are present. This adds conditional control flow to the pipeline, ensuring dump files are only published when available. Changes
Sequence Diagram(s)sequenceDiagram
participant Pipeline as Pipeline
participant Copy as CopyFiles@2
participant PS as PowerShell Script
participant Pub as PublishPipelineArtifact@1
Pipeline->>Copy: Execute file copy (from D:\a\_temp\ to $(Build.ArtifactStagingDirectory)/vstest_dumps)
Copy->>PS: Trigger PowerShell script (on succeededOrFailed())
PS->>PS: Inspect directory for dump files and set "hasFiles"
alt Dump files found
PS->>Pub: Initiate artifact publish
else No dump files
PS-->>Pub: Skip artifact publish
end
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
josesimoes
force-pushed
the
improve-assembly-dump
branch
from
9abd79a to
d261f5e
Compare
josesimoes
force-pushed
the
improve-assembly-dump
branch
5 times, most recently
from
61dcd98 to
bef2c78
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
azure-pipelines.yml (2)
219-227: Clarify Task Configuration for VSTest Dump File Copy.
The newCopyFiles@2task is configured to copy Visual Studio test dump files from a hard-coded temporary folder (D:\a\_temp\). Please verify that this path is consistent across all build agents and environments. Consider parameterizing the source folder if the location might change in the future, and confirm that flattening the folder structure (viaflattenFolders: true) meets your artifact organization requirements.
228-239: Normalize the Dump File Check Output.
In the PowerShell task that checks for dump files, the variablehasFilesis set as a boolean but is echoed directly. To ensure that the downstream condition (eq(variables['hasFiles'], 'true')) works correctly, it’s advisable to output a normalized lower-case string (for example, using$hasFiles.ToString().ToLower()). This will avoid potential mismatches betweenTrue/trueorFalse/false.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
MetadataProcessor.Shared/Extensions/TypeReferenceExtensions.csis excluded by none and included by noneMetadataProcessor.Shared/nanoDumperGenerator.csis excluded by none and included by noneMetadataProcessor.Tests/Core/ClrIntegrationTests.csis excluded by none and included by noneMetadataProcessor.Tests/Core/Extensions/TypeReferenceExtensionsTests.csis excluded by none and included by noneMetadataProcessor.Tests/Core/Utility/DumperTests.csis excluded by none and included by noneMetadataProcessor.Tests/MdpNFTestApp/MdpNFTestApp.slnis excluded by none and included by noneMetadataProcessor.Tests/TestNFApp/Program.csis excluded by none and included by none
📒 Files selected for processing (1)
azure-pipelines.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: nanoframework.metadata-processor (Build_MDP)
- GitHub Check: nanoframework.metadata-processor (Get_Build_Flags)
🔇 Additional comments (1)
azure-pipelines.yml (1)
240-247: Validate the Artifact Publishing Condition and Inputs.
ThePublishPipelineArtifact@1task uses the conditioneq(variables['hasFiles'], 'true')to determine whether to publish dump files. Please ensure that the output from the previous PowerShell task matches this expected lower-case string. Additionally, verify that the target path aligns with the output from the file copy task and that publishing under the artifact nameVsTestCrashDumpsis intentional.
- Output of locals, calls, strings, files and types now includes full names. - Update and improve unit tests as needed. - Replace Debug.Writeline with Console for NF test app. - Minor code style and fixes.
josesimoes
force-pushed
the
improve-assembly-dump
branch
from
bef2c78 to
0cc2ae9
Compare
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist:
Summary by CodeRabbit