Skip to content

Commit

Permalink
fix(perf): remove glob dependency
Browse files Browse the repository at this point in the history
  • Loading branch information
H4ad committed Apr 8, 2024
1 parent 836b800 commit c8af873
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 39 deletions.
40 changes: 30 additions & 10 deletions lib/utils/log-file.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
const os = require('os')
const { join, dirname, basename } = require('path')
const { format } = require('util')
const { glob } = require('glob')
const { Minipass } = require('minipass')
const fsMiniPass = require('fs-minipass')
const fs = require('fs/promises')
const log = require('./log-shim')
const Display = require('./display')

const padZero = (n, length) => n.toString().padStart(length.toString().length, '0')
const globify = pattern => pattern.split('\\').join('/')

class LogFiles {
// Default to a plain minipass stream so we can buffer
Expand Down Expand Up @@ -199,25 +197,47 @@ class LogFiles {

try {
const logPath = this.#getLogFilePath()
const logGlob = join(dirname(logPath), basename(logPath)
const patternFileName = basename(logPath)
// tell glob to only match digits
.replace(/\d/g, '[0123456789]')
.replace(/\d/g, 'd')
// Handle the old (prior to 8.2.0) log file names which did not have a
// counter suffix
.replace(/-\.log$/, '*.log')
)
.replace('-.log', '')

const files = await fs.readdir(
dirname(logPath), {
withFileTypes: true,
encoding: 'utf-8',
})
const logFiles = []

for (const file of files) {
if (!file.isFile()) {
continue
}

const genericFileName = file.name.replace(/\d/g, 'd')
const filePath = join(file.path, file.name)

// Always ignore the currently written files
if (
genericFileName.includes(patternFileName)
&& genericFileName.endsWith('.log')
&& !this.#files.includes(filePath)
) {
logFiles.push(filePath)
}
}

// Always ignore the currently written files
const files = await glob(globify(logGlob), { ignore: this.#files.map(globify), silent: true })
const toDelete = files.length - this.#logsMax
const toDelete = logFiles.length - this.#logsMax

if (toDelete <= 0) {
return
}

log.silly('logfile', `start cleaning logs, removing ${toDelete} files`)

for (const file of files.slice(0, toDelete)) {
for (const file of logFiles.slice(0, toDelete)) {
try {
await fs.rm(file, { force: true })
} catch (e) {
Expand Down
30 changes: 1 addition & 29 deletions test/lib/utils/log-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,35 +240,6 @@ t.test('doesnt need to clean', async t => {
t.equal(logs.length, oldLogs + 1)
})

t.test('glob error', async t => {
const { readLogs } = await loadLogFile(t, {
logsMax: 5,
mocks: {
glob: { glob: () => {
throw new Error('bad glob')
} },
},
})

const logs = await readLogs()
t.equal(logs.length, 1)
t.match(last(logs).content, /error cleaning log files .* bad glob/)
})

t.test('do not log cleaning errors when logging is disabled', async t => {
const { readLogs } = await loadLogFile(t, {
logsMax: 0,
mocks: {
glob: () => {
throw new Error('should not be logged')
},
},
})

const logs = await readLogs()
t.equal(logs.length, 0)
})

t.test('cleans old style logs too', async t => {
const logsMax = 5
const oldLogs = 10
Expand All @@ -290,6 +261,7 @@ t.test('rimraf error', async t => {
testdir: makeOldLogs(oldLogs),
mocks: {
'fs/promises': {
readdir: fs.readdir,
rm: async (...args) => {
if (count >= 3) {
throw new Error('bad rimraf')
Expand Down

0 comments on commit c8af873

Please sign in to comment.