Skip to content

Commit

Permalink
Merge pull request #1015 from ckeditor/ck/17025
Browse files Browse the repository at this point in the history
Fix (release-tools): Passing an odd number as a value of the `concurrency` option will not crash the `executeInParallel()` function. Closes ckeditor/ckeditor5#17025.
  • Loading branch information
pomek authored Sep 27, 2024
2 parents a090a62 + e21aaa7 commit 6127e07
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export default async function executeInParallel( options ) {
concurrency = os.cpus().length / 2
} = options;

const concurrencyAsInteger = Math.floor( concurrency ) || 1;
const normalizedCwd = upath.toUnix( cwd );
const packages = ( await glob( `${ packagesDirectory }/*/`, {
cwd: normalizedCwd,
Expand All @@ -58,7 +59,7 @@ export default async function executeInParallel( options ) {
packages.filter( packagesDirectoryFilter ) :
packages;

const packagesInThreads = getPackagesGroupedByThreads( packagesToProcess, concurrency );
const packagesInThreads = getPackagesGroupedByThreads( packagesToProcess, concurrencyAsInteger );

const callbackModule = upath.join( cwd, crypto.randomUUID() + '.mjs' );
await fs.writeFile( callbackModule, `export default ${ taskToExecute };`, 'utf-8' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { glob } from 'glob';
import fs from 'fs/promises';
import { registerAbortController, deregisterAbortController } from '../../lib/utils/abortcontroller.js';
import executeInParallel from '../../lib/utils/executeinparallel.js';
import os from 'os';

const stubs = vi.hoisted( () => ( {
WorkerMock: class {
Expand Down Expand Up @@ -334,6 +335,58 @@ describe( 'executeInParallel()', () => {
await promise;
} );

it( 'should use number of cores divided by two as default (`concurrency`)', async () => {
vi.mocked( os.cpus ).mockReturnValue( new Array( 7 ) );

const promise = executeInParallel( defaultOptions );
await delay( 0 );

expect( stubs.WorkerMock.instances ).toHaveLength( 3 );

// Workers did not emit an error.
for ( const worker of stubs.WorkerMock.instances ) {
getExitCallback( worker )( 0 );
}

await promise;
} );

it( 'should round down to the closest integer (`concurrency`)', async () => {
const options = Object.assign( {}, defaultOptions, {
concurrency: 3.5
} );

const promise = executeInParallel( options );
await delay( 0 );

expect( stubs.WorkerMock.instances ).toHaveLength( 3 );

// Workers did not emit an error.
for ( const worker of stubs.WorkerMock.instances ) {
getExitCallback( worker )( 0 );
}

await promise;
} );

it( 'should assign at least one thread even if concurrency is 0 (`concurrency`)', async () => {
const options = Object.assign( {}, defaultOptions, {
concurrency: 0
} );

const promise = executeInParallel( options );
await delay( 0 );

expect( stubs.WorkerMock.instances ).toHaveLength( 1 );

// Workers did not emit an error.
for ( const worker of stubs.WorkerMock.instances ) {
getExitCallback( worker )( 0 );
}

await promise;
} );

it( 'should resolve the promise if a worker finished (aborted) with a non-zero exit code (first worker)', async () => {
const promise = executeInParallel( defaultOptions );
await delay( 0 );
Expand Down

0 comments on commit 6127e07

Please sign in to comment.