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 3734005
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 41 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
52 changes: 21 additions & 31 deletions test/lib/utils/log-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ const loadLogFile = async (t, { buffer = [], mocks, testdir = {}, ...options } =
logFile,
LogFile,
readLogs: async () => {
const logDir = await fs.readdir(root)
const logFiles = logDir.map((f) => path.join(root, f))
const logDir = await fs.readdir(root, { withFileTypes: true })
const logFiles = logDir
.filter(f => f.isFile())
.map((f) => path.join(root, f.name))
.filter((f) => _fs.existsSync(f))
return Promise.all(logFiles.map(async (f) => {
const content = await fs.readFile(f, 'utf8')
Expand Down Expand Up @@ -202,6 +204,22 @@ t.test('cleans logs', async t => {
t.equal(logs.length, logsMax + 1)

Check failure on line 204 in test/lib/utils/log-file.js

View workflow job for this annotation

GitHub Actions / Test - Linux - 18.x

should be equal

Check failure on line 204 in test/lib/utils/log-file.js

View workflow job for this annotation

GitHub Actions / Test - macOS - 18.x

should be equal
})

t.test('cleans logs even when find folder inside logs folder', async t => {
const logsMax = 5
const { readLogs } = await loadLogFile(t, {
logsMax,
testdir: {
...makeOldLogs(10),
ignore_folder: {
'ignored-file.txt': 'hello',
},
},
})

const logs = await readLogs()
t.equal(logs.length, logsMax + 1)

Check failure on line 220 in test/lib/utils/log-file.js

View workflow job for this annotation

GitHub Actions / Test - Linux - 18.x

should be equal

Check failure on line 220 in test/lib/utils/log-file.js

View workflow job for this annotation

GitHub Actions / Test - macOS - 18.x

should be equal
})

t.test('doesnt clean current log by default', async t => {
const logsMax = 1
const { readLogs, logFile } = await loadLogFile(t, {
Expand Down Expand Up @@ -240,35 +258,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 +279,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 3734005

Please sign in to comment.