Skip to content

Commit e2554bc

Browse files
eps1loneps1lon
authored
[test] Let pending test finish on abort (#86307)
1 parent 0469aec commit e2554bc

File tree

1 file changed

+136
-110
lines changed

1 file changed

+136
-110
lines changed

‎run-tests.js‎

Lines changed: 136 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -501,10 +501,11 @@ ${ENDGROUP}`)
501501
`jest${process.platform === 'win32' ? '.CMD' : ''}`
502502
)
503503
let firstError = true
504-
let killed = false
504+
const testController = new AbortController()
505+
const testSignal = testController.signal
505506
let hadFailures = false
506507

507-
const runTest = (/** @type {TestFile} */ test, isFinalRun, isRetry) =>
508+
const runTestOnce = (/** @type {TestFile} */ test, isFinalRun, isRetry) =>
508509
new Promise((resolve, reject) => {
509510
const start = new Date().getTime()
510511
let outputChunks = []
@@ -618,11 +619,11 @@ ${ENDGROUP}`)
618619
if (hideOutput) {
619620
await outputSema.acquire()
620621
const isExpanded =
621-
firstError && !killed && !shouldContinueTestsOnError
622+
firstError && !testSignal.aborted && !shouldContinueTestsOnError
622623
if (isExpanded) {
623624
firstError = false
624625
process.stdout.write(`❌ ${test.file} output:\n`)
625-
} else if (killed) {
626+
} else if (testSignal.aborted) {
626627
process.stdout.write(`${GROUP}${test.file} output (killed)\n`)
627628
} else {
628629
process.stdout.write(`${GROUP}❌ ${test.file} output\n`)
@@ -636,7 +637,7 @@ ${ENDGROUP}`)
636637
output += chunk.toString()
637638
}
638639

639-
if (process.env.CI && !killed) {
640+
if (process.env.CI && !testSignal.aborted) {
640641
errorsPerTests.set(test.file, output)
641642
}
642643

@@ -682,10 +683,105 @@ ${ENDGROUP}`)
682683
})
683684
})
684685

686+
const runTest = async (/** @type {TestFile} */ test) => {
687+
let passed = false
688+
689+
const shouldSkipRetries = skipRetryTestManifest.find((t) =>
690+
t.includes(test.file)
691+
)
692+
const numRetries = shouldSkipRetries ? 0 : originalRetries
693+
if (shouldSkipRetries) {
694+
console.log(
695+
`Skipping retry for ${test.file} due to skipRetryTestManifest`
696+
)
697+
}
698+
699+
for (let i = 0; i < numRetries + 1; i++) {
700+
try {
701+
console.log(`Starting ${test.file} retry ${i}/${numRetries}`)
702+
const time = await runTestOnce(
703+
test,
704+
shouldSkipRetries || i === numRetries,
705+
shouldSkipRetries || i > 0
706+
)
707+
timings.push({
708+
file: test.file,
709+
time,
710+
})
711+
passed = true
712+
console.log(
713+
`${test.file} finished on retry ${i}/${numRetries} in ${time / 1000}s`
714+
)
715+
break
716+
} catch (err) {
717+
if (i < numRetries) {
718+
try {
719+
let testDir = path.dirname(path.join(__dirname, test.file))
720+
721+
// if test is nested in a test folder traverse up a dir to ensure
722+
// we clean up relevant test files
723+
if (testDir.endsWith('/test') || testDir.endsWith('\\test')) {
724+
testDir = path.join(testDir, '..')
725+
}
726+
console.log('Cleaning test files at', testDir)
727+
await exec(`git clean -fdx "${testDir}"`)
728+
await exec(`git checkout "${testDir}"`)
729+
} catch (err) {}
730+
} else {
731+
console.error(`${test.file} failed due to ${err}`)
732+
}
733+
}
734+
}
735+
736+
if (!passed) {
737+
hadFailures = true
738+
const error = new Error(
739+
// "failed to pass within" is a keyword parsed by next-pr-webhook
740+
`${test.file} failed to pass within ${numRetries} retries`
741+
)
742+
console.error(error.message)
743+
744+
if (!shouldContinueTestsOnError) {
745+
testController.abort(error)
746+
} else {
747+
console.log(
748+
`CONTINUE_ON_ERROR enabled, continuing tests after ${test.file} failed`
749+
)
750+
}
751+
}
752+
753+
// Emit test output if test failed or if we're continuing tests on error
754+
// This is parsed by the commenter webhook to notify about failing tests
755+
if ((!passed || shouldContinueTestsOnError) && isTestJob) {
756+
try {
757+
const testsOutput = await fsp.readFile(
758+
`${test.file}${RESULTS_EXT}`,
759+
'utf8'
760+
)
761+
const obj = JSON.parse(testsOutput)
762+
obj.processEnv = {
763+
NEXT_TEST_MODE: process.env.NEXT_TEST_MODE,
764+
HEADLESS: process.env.HEADLESS,
765+
}
766+
await outputSema.acquire()
767+
if (GROUP) console.log(`${GROUP}Result as JSON for tooling`)
768+
console.log(
769+
`--test output start--`,
770+
JSON.stringify(obj),
771+
`--test output end--`
772+
)
773+
if (ENDGROUP) console.log(ENDGROUP)
774+
outputSema.release()
775+
} catch (err) {
776+
console.log(`Failed to load test output`, err)
777+
}
778+
}
779+
}
780+
685781
const directorySemas = new Map()
686782

687783
const originalRetries = numRetries
688-
await Promise.all(
784+
const results = await Promise.allSettled(
689785
tests.map(async (test) => {
690786
const dirName = path.dirname(test.file)
691787
let dirSema = directorySemas.get(dirName)
@@ -695,109 +791,39 @@ ${ENDGROUP}`)
695791
if (/^test[/\\]integration/.test(test.file) && dirSema === undefined) {
696792
directorySemas.set(dirName, (dirSema = new Sema(1)))
697793
}
794+
// TODO: Use explicit resource managment instead of this acquire/release pattern
795+
// once CI runs with Node.js 24+.
698796
if (dirSema) await dirSema.acquire()
699-
700797
await sema.acquire()
701-
let passed = false
702798

703-
const shouldSkipRetries = skipRetryTestManifest.find((t) =>
704-
t.includes(test.file)
705-
)
706-
const numRetries = shouldSkipRetries ? 0 : originalRetries
707-
if (shouldSkipRetries) {
708-
console.log(
709-
`Skipping retry for ${test.file} due to skipRetryTestManifest`
710-
)
711-
}
712-
713-
for (let i = 0; i < numRetries + 1; i++) {
714-
try {
715-
console.log(`Starting ${test.file} retry ${i}/${numRetries}`)
716-
const time = await runTest(
717-
test,
718-
shouldSkipRetries || i === numRetries,
719-
shouldSkipRetries || i > 0
720-
)
721-
timings.push({
722-
file: test.file,
723-
time,
724-
})
725-
passed = true
726-
console.log(
727-
`Finished ${test.file} on retry ${i}/${numRetries} in ${
728-
time / 1000
729-
}s`
730-
)
731-
break
732-
} catch (err) {
733-
if (i < numRetries) {
734-
try {
735-
let testDir = path.dirname(path.join(__dirname, test.file))
736-
737-
// if test is nested in a test folder traverse up a dir to ensure
738-
// we clean up relevant test files
739-
if (testDir.endsWith('/test') || testDir.endsWith('\\test')) {
740-
testDir = path.join(testDir, '..')
741-
}
742-
console.log('Cleaning test files at', testDir)
743-
await exec(`git clean -fdx "${testDir}"`)
744-
await exec(`git checkout "${testDir}"`)
745-
} catch (err) {}
746-
} else {
747-
console.error(`${test.file} failed due to ${err}`)
748-
}
749-
}
750-
}
751-
752-
if (!passed) {
753-
console.error(
754-
`${test.file} failed to pass within ${numRetries} retries`
755-
)
756-
757-
if (!shouldContinueTestsOnError) {
758-
killed = true
759-
children.forEach((child) => child.kill())
760-
cleanUpAndExit(1)
761-
} else {
762-
hadFailures = true
763-
console.log(
764-
`CONTINUE_ON_ERROR enabled, continuing tests after ${test.file} failed`
765-
)
799+
try {
800+
if (testSignal.aborted) {
801+
// We already logged the abort reason. No need to include it in cause.
802+
const error = new Error(`Skipped due to abort.`)
803+
error.name = test.file
804+
throw error
766805
}
767-
}
768806

769-
// Emit test output if test failed or if we're continuing tests on error
770-
// This is parsed by the commenter webhook to notify about failing tests
771-
if ((!passed || shouldContinueTestsOnError) && isTestJob) {
772-
try {
773-
const testsOutput = await fsp.readFile(
774-
`${test.file}${RESULTS_EXT}`,
775-
'utf8'
776-
)
777-
const obj = JSON.parse(testsOutput)
778-
obj.processEnv = {
779-
NEXT_TEST_MODE: process.env.NEXT_TEST_MODE,
780-
HEADLESS: process.env.HEADLESS,
781-
}
782-
await outputSema.acquire()
783-
if (GROUP) console.log(`${GROUP}Result as JSON for tooling`)
784-
console.log(
785-
`--test output start--`,
786-
JSON.stringify(obj),
787-
`--test output end--`
788-
)
789-
if (ENDGROUP) console.log(ENDGROUP)
790-
outputSema.release()
791-
} catch (err) {
792-
console.log(`Failed to load test output`, err)
793-
}
807+
await runTest(test)
808+
} finally {
809+
sema.release()
810+
if (dirSema) dirSema.release()
794811
}
795-
796-
sema.release()
797-
if (dirSema) dirSema.release()
798812
})
799813
)
800814

815+
for (const result of results) {
816+
if (result.status === 'rejected') {
817+
hadFailures = true
818+
console.error(result.reason)
819+
}
820+
}
821+
822+
if (hadFailures && !shouldContinueTestsOnError) {
823+
// TODO: Does it make sense to update timings if there were failures if without shouldContinueTestsOnError?
824+
return hadFailures
825+
}
826+
801827
if (options.timings) {
802828
const curTimings = {}
803829
// let junitData = `<testsuites name="jest tests">`
@@ -857,20 +883,20 @@ ${ENDGROUP}`)
857883
}
858884
}
859885

860-
// Return whether there were any failures
861886
return hadFailures
862887
}
863888

864-
main()
865-
.then((hadFailures) => {
889+
main().then(
890+
(hadFailures) => {
866891
if (hadFailures) {
867892
console.error('Some tests failed')
868-
cleanUpAndExit(1)
893+
return cleanUpAndExit(1)
869894
} else {
870-
cleanUpAndExit(0)
895+
return cleanUpAndExit(0)
871896
}
872-
})
873-
.catch((err) => {
874-
console.error(err)
875-
cleanUpAndExit(1)
876-
})
897+
},
898+
(reason) => {
899+
console.error(reason)
900+
return cleanUpAndExit(1)
901+
}
902+
)

0 commit comments

Comments
 (0)