Skip to content

Conversation

@sarnex
Copy link
Contributor

@sarnex sarnex commented Jan 31, 2025

Obviously we shouldn't have to do this, but these hang issues are causing serious CI issues and annoying users.

If there is a hung test detected, CI will fail after killing the process, so we should still catch the hung tests.

I can't add an action instead of downloading the script because you need to do a (cached) checkout to get the action file and that tries to reset the git repo which fails if there's locked files.

Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex marked this pull request as ready for review February 3, 2025 15:34
@sarnex sarnex requested a review from a team as a code owner February 3, 2025 15:34
Comment on lines 83 to 85
Invoke-WebRequest -Uri "https://raw.githubusercontent.com/intel/llvm/refs/heads/sycl/devops/scripts/windows_detect_hung_tests.ps1" -OutFile "windows_detect_hung_tests.ps1"
powershell.exe -File windows_detect_hung_tests.ps1
exit $LASTEXITCODE
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should NOT result in any failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

couple of points here

  1. the single place we call it today already does
  2. im worried we will never catch hung tests if we dont fail because otherwise the only way to catch it would be to manually read the ci logs

environment: WindowsCILock
env: ${{ fromJSON(inputs.env) }}
steps:
- name: Detect hung tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments apply to this file.

Signed-off-by: Sarnie, Nick <[email protected]>
Signed-off-by: Sarnie, Nick <[email protected]>
Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

LGTM

@sarnex sarnex merged commit 7025de8 into intel:sycl Feb 3, 2025
5 checks passed
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.

3 participants