Skip to content

Commit

Permalink
refactor: make hasPowerShell3 async, removing execFileSync
Browse files Browse the repository at this point in the history
fix #24
  • Loading branch information
Alex-D committed May 21, 2023
1 parent 1fc9201 commit e66917a
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 45 deletions.
22 changes: 10 additions & 12 deletions src/functions/hasPowerShell3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,20 @@ import Dependencies from '@/src/types/dependencies'
* Note: 6.* is Windows 7
* Note: PowerShell 3 is natively available since Windows 8
*
* @param release - OS Release number
* @param dependencies - Dependencies Injection Container
*/
function hasPowerShell3(dependencies: Dependencies): boolean {
function hasPowerShell3(dependencies: Dependencies): Promise<boolean> {
const major = parseInt(dependencies.release.split('.')[0], 10)

if (major > 6)
{
try {
dependencies.cpExecFileSync('powershell')
return true
} catch (err) {
return false
}
} else {
return false
if (major <= 6) {
return Promise.resolve(false)
}

return new Promise<boolean>(function (resolve) {
dependencies.cpExecFile('where', ['powershell'], { windowsHide: true }, function (error) {
resolve(!error)
})
})
}

export default hasPowerShell3
19 changes: 7 additions & 12 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { execFile, execFileSync } from 'child_process'
import { execFile } from 'child_process'
import { existsSync } from 'fs'
import { release } from 'os'
import { normalize, sep } from 'path'
Expand All @@ -18,13 +18,12 @@ import DiskSpace from '@/src/types/diskSpace'
* @param dependencies - Dependencies container
*/
function checkDiskSpace(directoryPath: string, dependencies: Dependencies = {
platform: platform,
platform,
release: release(),
fsExistsSync: existsSync,
pathNormalize: normalize,
pathSep: sep,
cpExecFile: execFile,
cpExecFileSync: execFileSync,
}): Promise<DiskSpace> {
// Note: This function contains other functions in order
// to wrap them in a common context and make unit tests easier
Expand Down Expand Up @@ -87,7 +86,7 @@ function checkDiskSpace(directoryPath: string, dependencies: Dependencies = {
return Promise.reject(new Error('cmd must contain at least one item'))
}

dependencies.cpExecFile(file, args, (error, stdout) => {
dependencies.cpExecFile(file, args, { windowsHide: true }, (error, stdout) => {
if (error) {
reject(error)
}
Expand All @@ -106,11 +105,9 @@ function checkDiskSpace(directoryPath: string, dependencies: Dependencies = {
*
* @param directoryPath - The file/folder path from where we want to know disk space
*/
function checkWin32(directoryPath: string): Promise<DiskSpace> {
async function checkWin32(directoryPath: string): Promise<DiskSpace> {
if (directoryPath.charAt(1) !== ':') {
return new Promise((resolve, reject) => {
reject(new InvalidPathError(`The following path is invalid (should be X:\\...): ${directoryPath}`))
})
return Promise.reject(new InvalidPathError(`The following path is invalid (should be X:\\...): ${directoryPath}`))
}

const powershellCmd = [
Expand All @@ -123,7 +120,7 @@ function checkDiskSpace(directoryPath: string, dependencies: Dependencies = {
'get',
'size,freespace,caption',
]
const cmd = hasPowerShell3(dependencies) ? powershellCmd : wmicCmd
const cmd = await hasPowerShell3(dependencies) ? powershellCmd : wmicCmd

return check(
cmd,
Expand All @@ -147,9 +144,7 @@ function checkDiskSpace(directoryPath: string, dependencies: Dependencies = {
*/
function checkUnix(directoryPath: string): Promise<DiskSpace> {
if (!dependencies.pathNormalize(directoryPath).startsWith(dependencies.pathSep)) {
return new Promise((resolve, reject) => {
reject(new InvalidPathError(`The following path is invalid (should start by ${dependencies.pathSep}): ${directoryPath}`))
})
return Promise.reject(new InvalidPathError(`The following path is invalid (should start by ${dependencies.pathSep}): ${directoryPath}`))
}

const pathToCheck = getFirstExistingParentPath(directoryPath, dependencies)
Expand Down
14 changes: 12 additions & 2 deletions src/types/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,18 @@ type Dependencies = {
fsExistsSync: typeof existsSync
pathNormalize: typeof normalize
pathSep: typeof sep
cpExecFile: (file: string, args: ReadonlyArray<string> | undefined | null, callback: (error: ExecFileException | null, stdout: string, stderr: string) => void) => ChildProcess
cpExecFileSync: (command: string) => Buffer
cpExecFile: (
file: string,
args: ReadonlyArray<string> | undefined | null,
options: {
windowsHide: true
},
callback: (
error: ExecFileException | null,
stdout: string,
stderr: string
) => void
) => ChildProcess
}

export default Dependencies
10 changes: 1 addition & 9 deletions test/__helpers__/mockDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ import Dependencies from '@/src/types/dependencies'
function mockDependencies(overrides?: Partial<Dependencies>, options?: {
cpExecFileOutput?: string
cpExecFileError?: Error
cpExecFileSyncError?: Error
}): Dependencies {
const dependencies: Dependencies = {
platform: 'linux',
release: '11.5.0',
fsExistsSync: () => true,
pathNormalize: normalize,
pathSep: '/',
cpExecFile: (cmd, args, callback) => {
cpExecFile: (cmd, args, opts, callback) => {
process.nextTick(() => {
if (options?.cpExecFileError !== undefined) {
callback(options.cpExecFileError, '', '')
Expand All @@ -26,13 +25,6 @@ function mockDependencies(overrides?: Partial<Dependencies>, options?: {

return new EventEmitter() as ChildProcess
},
cpExecFileSync: () => {
if (options?.cpExecFileSyncError !== undefined) {
throw options.cpExecFileSyncError
}

return Buffer.from('')
},
...overrides,
}

Expand Down
20 changes: 10 additions & 10 deletions test/functions/hasPowerShell3.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,50 @@ import hasPowerShell3 from '@/src/functions/hasPowerShell3'
import mockDependencies from '@/test/__helpers__/mockDependencies'


test('windows: release <=6 must NOT have PowerShell 3', t => {
test('windows: release <=6 must NOT have PowerShell 3', async t => {
const dependencies = mockDependencies({
platform: 'win32',
})

t.is(hasPowerShell3({
t.is(await hasPowerShell3({
...dependencies,
release: '5.0.12',
}), false)

t.is(hasPowerShell3({
t.is(await hasPowerShell3({
...dependencies,
release: '6.1.0',
}), false)
})

test('windows: release 7+ must have PowerShell 3', t => {
test('windows: release 7+ must have PowerShell 3', async t => {
const dependencies = mockDependencies({
platform: 'win32',
})

t.is(hasPowerShell3({
t.is(await hasPowerShell3({
...dependencies,
release: '7.3.15',
}), true)

t.is(hasPowerShell3({
t.is(await hasPowerShell3({
...dependencies,
release: '10.3.15',
}), true)

t.is(hasPowerShell3({
t.is(await hasPowerShell3({
...dependencies,
release: '11.14.0',
}), true)
})

test('windows: release 7+ powershell ENOENT', t => {
test('windows: release 7+ powershell ENOENT', async t => {
const dependencies = mockDependencies({
platform: 'win32',
release: '11.14.0',
}, {
cpExecFileSyncError: new Error('ENOENT'),
cpExecFileError: new Error('ENOENT'),
})

t.is(hasPowerShell3(dependencies), false)
t.is(await hasPowerShell3(dependencies), false)
})

0 comments on commit e66917a

Please sign in to comment.