OSOE-464: Executing dotnet test with process timeout

[dministro]

OSOE-464

The primary goal here is to detach the stdout of the dotnet test process to avoid the issue described here, and at the same time, we have the possibility to control the process timeout. However the blame-hang-timeout does a very similar thing, as I have seen in the logs the dotnet test process is still stuck while the Data collector 'Blame' message: All tests finished running, Sequence file will not be generated. message was recorded, this means something is happening outside the blame controller scope.

Summary by CodeRabbit

• New Features

  • Implemented a timeout for .NET test processes with the ability to collect diagnostics if a hang occurs.

• Documentation

  • Updated workflow files to document the new dotnet-test-process-timeout input parameter.

• Chores

  • Added steps for installing dotnet-dump and uploading diagnostic dumps for test hangs.

• Bug Fixes

  • Introduced a mechanism to handle test process execution and prevent indefinite hangs during automated testing.

[Piedone]

@coderabbitai review

I'm testing the AI code review tool CodeRabbit here. Please bear with me. And if it comments some useful feedback, please address it.

[coderabbitai]

Walkthrough

Walkthrough

The recent updates across various GitHub action and workflow files introduce a new parameter, $TestProcessTimeout, to manage the timeout of the dotnet test process. This enhancement allows for the specification of a timeout value, aiding in handling cases where the test process may hang. The scripts and workflows have been improved to start and monitor the test process with this timeout consideration. Additionally, there's the inclusion of dotnet-dump for capturing dumps if timeouts occur, and an artifact upload step has been added to save these dumps under certain conditions.

Changes

File	Change Summary
.github/actions/test-dotnet/Invoke-SolutionTests.ps1	Modified Invoke-SolutionTests.ps1 to include new parameter $TestProcessTimeout and several new functions related to process management and timeout handling. Also, added new logic to handle process timeouts and successful test runs, along with new variables and modification of the function StartProcessAndWaitForExit to include the $Timeout parameter.
.github/actions/test-dotnet/action.yml	Added dotnet-test-process-timeout input, dotnet-dump installation step, and updated Run Tests step to pass TestProcessTimeout. Additionally, included a conditional DotnetTestHangDumps artifact upload step.
.github/workflows/build-and-test-dotnet.yml .github/workflows/build-and-test-orchard-core.yml .github/workflows/msbuild-and-test.yml	Introduced dotnet-test-process-timeout parameter with default and description. Updated test-dotnet action usage to include the new timeout parameter and specific version tag in the case of msbuild-and-test.yml.

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

@Piedone, I'm ready to review the pull request as requested. Please provide the specific files or code snippets you would like me to review, or let me know if you want a general review of the entire pull request.

Action performed

Review triggered.

[Piedone]

@coderabbitai review

Sorry about that.

[coderabbitai]

@Piedone, no problem at all! I'm here to assist you. If you could specify which parts of the pull request you'd like me to review or if you want a general review, I'll get started right away. Just let me know how I can help!

Action performed

Review triggered.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 9c3f9ec and fc07590.

Files selected for processing (4)

• .github/actions/test-dotnet/Invoke-SolutionTests.ps1 (3 hunks)
• .github/actions/test-dotnet/action.yml (4 hunks)
• .github/workflows/build-and-test-dotnet.yml (2 hunks)
• .github/workflows/build-and-test-orchard-core.yml (2 hunks)

Additional comments: 10

.github/actions/test-dotnet/Invoke-SolutionTests.ps1 (2)

• 1-1: The addition of the $TestProcessTimeout parameter to the script is noted and appears to be correctly implemented.

• 51-121: The StartProcessAndWaitForExit function is well-structured and includes comprehensive handling for process execution with a timeout, including collecting a dump if the process hangs. Ensure that the $Timeout parameter is being passed correctly wherever this function is called.

.github/actions/test-dotnet/action.yml (4)

• 18-22: The new input dotnet-test-process-timeout is added with a default value of -1 and a description, which aligns with the PR objectives.

• 88-92: The addition of the step to install dotnet-dump with a specific version is appropriate for collecting dumps of hanging test processes.

• 105-105: The Run Tests step correctly utilizes the TestProcessTimeout parameter, ensuring the new timeout functionality is applied during test execution.

• 146-153: The new step to upload DotnetTestHangDumps is well-configured with appropriate conditions and retention settings.

.github/workflows/build-and-test-dotnet.yml (2)

• 103-107: The new input parameter dotnet-test-process-timeout is added to the workflow file with a default value of -1 and a description, which is consistent with the PR objectives.

• 197-197: The Tests job correctly passes the dotnet-test-process-timeout parameter to the test-dotnet action, integrating the new timeout functionality into the workflow.

.github/workflows/build-and-test-orchard-core.yml (2)

• 104-108: The addition of the dotnet-test-process-timeout input parameter with a default value of -1 is consistent with the PR objectives. Ensure that the consuming actions and scripts are updated to handle the case when the timeout is set to -1, which likely indicates no timeout.

• 241-245: The dotnet-test-process-timeout parameter is correctly passed to the test-dotnet action. Verify that the test-dotnet action at the branch or tag issue/OSOE-464-executing-dotnet-test-with-process-timeout is implemented to accept and correctly handle this new parameter.

Verification successful

---

The output from the shell script confirms that the dotnet-test-process-timeout parameter is indeed present in the action.yml file of the test-dotnet action. It also shows that the TestProcessTimeout is being set with the value from inputs.dotnet-test-process-timeout, which indicates that the action is designed to accept and use this new parameter.

Based on this information, the review comment is verified as correct:

• The dotnet-test-process-timeout parameter is properly passed to the test-dotnet action.
• The test-dotnet action is implemented to accept and handle the dotnet-test-process-timeout parameter.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Verify that the test-dotnet action supports the new dotnet-test-process-timeout parameter.
rg --type yaml 'dotnet-test-process-timeout' .github/actions/test-dotnet/action.yml

Length of output: 187

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between fc07590 and 888f9ef.

Files selected for processing (3)

• .github/actions/test-dotnet/Invoke-SolutionTests.ps1 (5 hunks)
• .github/workflows/build-and-test-dotnet.yml (2 hunks)
• .github/workflows/build-and-test-orchard-core.yml (2 hunks)

Additional comments: 12

.github/actions/test-dotnet/Invoke-SolutionTests.ps1 (8)

• 1-1: The addition of $TestProcessTimeout parameter is consistent with the PR objectives to introduce a timeout mechanism for the test process.

• 73-76: The GetChildProcesses function is a utility to fetch child processes of a given process ID. Ensure that this function is used consistently throughout the script.

• 78-88: The DumpProcess function is designed to collect a dump of a process. It's important to verify that the dotnet-dump tool is installed and available in the environment where this script will run.

• 90-98: The DumpProcessTree function recursively collects dumps of a process and its child processes. This is a good addition for debugging purposes.

• 100-110: The KillProcessTree function recursively kills a process and its child processes. It's crucial to ensure that this function is called only when necessary to avoid unintended terminations.

• 112-173: The StartProcessAndWaitForExit function encapsulates the logic to start a process and wait for its exit with a timeout. This is the core of the timeout feature implementation. It's important to ensure that the $Timeout parameter is being passed correctly and that the function handles both the process exiting naturally and being killed due to a timeout.

• 187-189: The addition of detailed logging with the trx and console loggers will help in diagnosing test failures and understanding test execution flow.

• 198-208: The logic to handle the test process result seems correct. It accounts for both successful completion and forced termination due to a timeout. However, ensure that the $process.Id variable is defined before it's used in the warning message on line 206.

.github/workflows/build-and-test-dotnet.yml (2)

• 104-108: The new dotnet-test-process-timeout parameter has been added with a default value of -1, which is consistent with the PR objectives to allow specifying a timeout for the test process.

• 217-217: The dotnet-test-process-timeout parameter is correctly passed to the test-dotnet action. This ensures that the timeout functionality is available during the test execution step.

.github/workflows/build-and-test-orchard-core.yml (2)

• 102-106: The addition of the dotnet-test-process-timeout parameter to the workflow file is consistent with the changes made in the build-and-test-dotnet.yml file and aligns with the PR objectives.

• 253-257: The usage of the dotnet-test-process-timeout parameter in the test-dotnet action is correctly implemented, mirroring the changes in the build-and-test-dotnet.yml file.

[Piedone]

So, this is a speculative but not necessarily definitive fix, as well as a debugging aid if processes still hang, right?

[dministro]

So, this is a speculative but not necessarily definitive fix, as well as a debugging aid if processes still hang, right?

After fixing some deadlocks, I didn't find locking or similar stuff in the memory dumps anymore, and the blame hang timeout doesn't help, only theories are left and yes this change creates memory dumps for the processes in the process tree to help debug if the issue still remains. We will see it.

[Piedone]

OK, thanks.