Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill test process if it takes more than 5 minutes to execute #2454

Merged
merged 2 commits into from
May 12, 2023

Conversation

Alan-Jowett
Copy link
Member

Description

During fault injection, set a watchdog timer in PS to kill the process if it takes more than 5 minutes to execute.

Testing

CI/CD

Documentation

No.

@@ -22,6 +22,16 @@ Set-Content -Path ($TestProgram + ".fault.log") ""

$iteration = 0

$timer = New-Object System.Timers.Timer
# Set timer for 5 minutes
$timer.Interval = 300000
Copy link
Collaborator

@shpalani shpalani May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this time value even for fault_injection_full?

Does fault_injection_full take more than 5 mins?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't. Each iteration should take less than 5 minutes unless it's stuck.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. The reason of ask is, we have more APIs added with fault injection.

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #2454 (06bc5c5) into main (d0db507) will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2454      +/-   ##
==========================================
+ Coverage   83.96%   84.03%   +0.07%     
==========================================
  Files         156      156              
  Lines       29053    29092      +39     
==========================================
+ Hits        24393    24447      +54     
+ Misses       4660     4645      -15     

see 20 files with indirect coverage changes


# When the watchdog timer fires, kill the test binary.
Register-ObjectEvent -InputObject $timer -EventName Elapsed -Action {
Write-Error "Test binary exceede time"
Copy link
Collaborator

@shpalani shpalani May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct spelling: exceeded.

Also, will the user know which binary or test case exceeded the time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should gather a memory dump of the hung process, but that requires some additional work. The first version is to improve the reliability of the CI/CD pipeline.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have an issue tracking the improvement for the next step?

@@ -22,6 +22,16 @@ Set-Content -Path ($TestProgram + ".fault.log") ""

$iteration = 0

$timer = New-Object System.Timers.Timer
# Set timer for 5 minutes
$timer.Interval = 300000
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. The reason of ask is, we have more APIs added with fault injection.

saxena-anurag
saxena-anurag previously approved these changes May 10, 2023
# Set timer for 5 minutes
$timer.Interval = 300000

# When the watchdog timer fires, kill the test binary.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And will this be treated as a pass or a failure?

@saxena-anurag saxena-anurag added this pull request to the merge queue May 12, 2023
Merged via the queue into microsoft:main with commit f8f413a May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants