-
-
Notifications
You must be signed in to change notification settings - Fork 363
chore(ci): Add script to cancel SauceLabs jobs #5873
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
base: main
Are you sure you want to change the base?
Conversation
…tion - Implemented a retry mechanism for failed SauceLabs tests based on output logs. - Added a new script `saucelabs-helpers.js` for managing SauceLabs jobs, including functions to check if tests should be retried and to cancel jobs. - Updated the GitHub Actions workflow to utilize the new helper functions for improved error handling and resource management during test execution.
cursor review |
|
||
try { | ||
const shouldRetry = await shouldRetryTest(args[1]); | ||
process.exit(shouldRetry ? 0 : 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Boolean Check Error in Test Recovery Function
The runCheckTestRecovery
function incorrectly uses the object returned by shouldRetryTest()
as a boolean. Since JavaScript objects are always truthy, process.exit(shouldRetry ? 0 : 2)
will always result in an exit code of 0, regardless of the actual shouldRetry
boolean value within the returned object. The code should access the shouldRetry
property of the returned object.
console.log(`Log file ${logFile} not found, skipping...`); | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Function Exits Prematurely on Missing Files
The extractJobIdsFromLogs
function incorrectly uses return;
instead of continue;
when a log file does not exist. This causes the function to exit prematurely, preventing the processing of any remaining log files in the input array. Consequently, the function returns undefined
instead of a Set
, leading to errors in calling code that expects a Set
object.
const path = require('path'); | ||
|
||
const outputLogPath = path.join(__dirname, "output.log"); | ||
const retryOutputLogPath = path.join(__dirname, "retry-output.log"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: GitHub Actions Script Path Resolution Error
The __dirname
variable in the actions/github-script
environment does not correctly resolve to the workflow's working directory. This causes incorrect paths when attempting to access log files like output.log
and retry-output.log
, which are created in the working directory. process.cwd()
or relative paths should be used instead.
The current implementation of the benchmark workflows is triggering jobs on SauceLabs incl. recovery on infrastructure errors.
When the GitHub Action job is cancelled, it does not cancel the SauceLabs job, leading a waste of resources.
As the SauceLabs CLI is quite limited (can't get jobs or cancel them), this PR adds a JavaScript based helper to implement the missing functionality of retry-logic and cancelling of jobs.
#skip-changelog