test(log): extract log-rotation into testable module + 10 unit tests
The fileuploader.log rotation introduced in 3.3.2 lived inline in main.js — fine for the runtime path, but it required electron's `app` to even reach the function under test. Pull the rotation logic into lib/log-rotation.js (pure fs/path, no electron deps) and cover it properly: - ENOENT (file missing) → no-op - Below cap → no-op - Over cap → live → .1, returns true - Existing backups shift up: .1 → .2, .2 → .3 - At maxBackups limit → oldest dropped, others shift, live becomes .1 - Idempotent: rotating twice keeps the chain consistent - maxBackups=1: never grows past .1 - Invalid maxBytes (0/negative/NaN) → safe no-op - Provided debug callback receives a "rotated" message - File without extension still rotates correctly main.js now imports `maybeRotateLogFile` and calls it directly. 97/97 tests pass.
This commit is contained in:
parent
79fe41c774
commit
d9c3a00016
52
lib/log-rotation.js
Normal file
52
lib/log-rotation.js
Normal file
@ -0,0 +1,52 @@
|
|||||||
|
// Generic numbered-backup log rotation. Used by the upload log + can be
|
||||||
|
// reused by other long-lived log files (debug log, account-rotation log).
|
||||||
|
//
|
||||||
|
// Behaviour:
|
||||||
|
// - File missing → no-op, returns false.
|
||||||
|
// - File ≤ maxBytes → no-op, returns false.
|
||||||
|
// - File > maxBytes → drop oldest .N backup, shift .K → .K+1, rename live
|
||||||
|
// file to .1, return true. Caller (or the next append) creates a fresh
|
||||||
|
// primary on demand.
|
||||||
|
//
|
||||||
|
// Errors are reported via `log` (e.g. debugLog) but never thrown — rotation
|
||||||
|
// is best-effort; the caller's append happens anyway.
|
||||||
|
|
||||||
|
const fs = require('fs');
|
||||||
|
const path = require('path');
|
||||||
|
|
||||||
|
function maybeRotateLogFile(filePath, maxBytes, maxBackups = 3, log = () => {}) {
|
||||||
|
if (!filePath || !Number.isFinite(maxBytes) || maxBytes <= 0) return false;
|
||||||
|
let size = 0;
|
||||||
|
try {
|
||||||
|
const st = fs.statSync(filePath);
|
||||||
|
size = st.size;
|
||||||
|
} catch (err) {
|
||||||
|
// ENOENT is normal — nothing to rotate yet.
|
||||||
|
if (err && err.code !== 'ENOENT') {
|
||||||
|
log(`logRotation: stat ${filePath} failed: ${err.message}`);
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (size <= maxBytes) return false;
|
||||||
|
|
||||||
|
const ext = path.extname(filePath);
|
||||||
|
const base = filePath.slice(0, filePath.length - ext.length);
|
||||||
|
|
||||||
|
// Drop the oldest backup if it exists, then shift each numbered backup up
|
||||||
|
// one slot. Errors are ignored: missing intermediate backups are normal,
|
||||||
|
// failed renames just mean we'll rotate again next time.
|
||||||
|
try { fs.unlinkSync(`${base}.${maxBackups}${ext}`); } catch {}
|
||||||
|
for (let i = maxBackups - 1; i >= 1; i--) {
|
||||||
|
try { fs.renameSync(`${base}.${i}${ext}`, `${base}.${i + 1}${ext}`); } catch {}
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
fs.renameSync(filePath, `${base}.1${ext}`);
|
||||||
|
log(`logRotation: rotated ${filePath} (${(size / 1024 / 1024).toFixed(1)} MB) → ${base}.1${ext}`);
|
||||||
|
return true;
|
||||||
|
} catch (err) {
|
||||||
|
log(`logRotation: rename ${filePath} → ${base}.1${ext} failed: ${err.message}`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = { maybeRotateLogFile };
|
||||||
31
main.js
31
main.js
@ -13,6 +13,7 @@ const { checkForUpdate, installUpdate, abortUpdate } = require('./lib/updater');
|
|||||||
const backupCrypto = require('./lib/backup-crypto');
|
const backupCrypto = require('./lib/backup-crypto');
|
||||||
const FolderMonitor = require('./lib/folder-monitor');
|
const FolderMonitor = require('./lib/folder-monitor');
|
||||||
const RemoteServer = require('./lib/remote-server');
|
const RemoteServer = require('./lib/remote-server');
|
||||||
|
const { maybeRotateLogFile } = require('./lib/log-rotation');
|
||||||
|
|
||||||
let mainWindow;
|
let mainWindow;
|
||||||
let _lastImportPath = null;
|
let _lastImportPath = null;
|
||||||
@ -315,34 +316,6 @@ function _resolveUploadLogTarget() {
|
|||||||
const UPLOAD_LOG_MAX_BYTES = 50 * 1024 * 1024;
|
const UPLOAD_LOG_MAX_BYTES = 50 * 1024 * 1024;
|
||||||
const UPLOAD_LOG_MAX_BACKUPS = 3;
|
const UPLOAD_LOG_MAX_BACKUPS = 3;
|
||||||
|
|
||||||
function _maybeRotateUploadLog(filePath) {
|
|
||||||
let size = 0;
|
|
||||||
try {
|
|
||||||
const st = fs.statSync(filePath);
|
|
||||||
size = st.size;
|
|
||||||
} catch (err) {
|
|
||||||
// ENOENT is the common case (file doesn't exist yet) — nothing to rotate.
|
|
||||||
if (err && err.code !== 'ENOENT') {
|
|
||||||
debugLog(`uploadLog stat for rotation failed: ${err.message}`);
|
|
||||||
}
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
if (size <= UPLOAD_LOG_MAX_BYTES) return;
|
|
||||||
const ext = path.extname(filePath);
|
|
||||||
const base = filePath.slice(0, filePath.length - ext.length);
|
|
||||||
// Drop the oldest backup if it exists, then shift each numbered backup up.
|
|
||||||
try { fs.unlinkSync(`${base}.${UPLOAD_LOG_MAX_BACKUPS}${ext}`); } catch {}
|
|
||||||
for (let i = UPLOAD_LOG_MAX_BACKUPS - 1; i >= 1; i--) {
|
|
||||||
try { fs.renameSync(`${base}.${i}${ext}`, `${base}.${i + 1}${ext}`); } catch {}
|
|
||||||
}
|
|
||||||
try {
|
|
||||||
fs.renameSync(filePath, `${base}.1${ext}`);
|
|
||||||
debugLog(`uploadLog rotated (${(size / 1024 / 1024).toFixed(1)} MB) → ${base}.1${ext}`);
|
|
||||||
} catch (err) {
|
|
||||||
debugLog(`uploadLog rotation rename failed: ${err.message}`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
function _flushUploadLog() {
|
function _flushUploadLog() {
|
||||||
if (_uploadLogWriting || _uploadLogBuffer.length === 0) return;
|
if (_uploadLogWriting || _uploadLogBuffer.length === 0) return;
|
||||||
const target = _resolveUploadLogTarget();
|
const target = _resolveUploadLogTarget();
|
||||||
@ -354,7 +327,7 @@ function _flushUploadLog() {
|
|||||||
try { fs.mkdirSync(path.dirname(target.path), { recursive: true }); } catch {}
|
try { fs.mkdirSync(path.dirname(target.path), { recursive: true }); } catch {}
|
||||||
// Cheap size check + rotation right before the append, so we never grow
|
// Cheap size check + rotation right before the append, so we never grow
|
||||||
// a single log file beyond the cap regardless of session length.
|
// a single log file beyond the cap regardless of session length.
|
||||||
_maybeRotateUploadLog(target.path);
|
maybeRotateLogFile(target.path, UPLOAD_LOG_MAX_BYTES, UPLOAD_LOG_MAX_BACKUPS, debugLog);
|
||||||
const chunk = _uploadLogBuffer.join('');
|
const chunk = _uploadLogBuffer.join('');
|
||||||
_uploadLogBuffer.length = 0;
|
_uploadLogBuffer.length = 0;
|
||||||
_uploadLogWriting = true;
|
_uploadLogWriting = true;
|
||||||
|
|||||||
@ -6,11 +6,13 @@
|
|||||||
- ✅ 3.3.2 — `fileuploader.log` Auto-Rotation bei 50 MB (max 3 Backups: .1 .2 .3)
|
- ✅ 3.3.2 — `fileuploader.log` Auto-Rotation bei 50 MB (max 3 Backups: .1 .2 .3)
|
||||||
- ✅ 3.3.3 — `_jobLogCollector` Cap auf 1000 tracked jobs (FIFO-eviction beim Überschreiten)
|
- ✅ 3.3.3 — `_jobLogCollector` Cap auf 1000 tracked jobs (FIFO-eviction beim Überschreiten)
|
||||||
- ✅ 3.3.4 — `applyQueueSelectionClasses` + `applyRecentSelectionClasses` nutzen `getElementsByClassName` (live HTMLCollection statt querySelectorAll re-query bei jedem Klick)
|
- ✅ 3.3.4 — `applyQueueSelectionClasses` + `applyRecentSelectionClasses` nutzen `getElementsByClassName` (live HTMLCollection statt querySelectorAll re-query bei jedem Klick)
|
||||||
|
- ✅ 3.3.5 — Log-Rotation extrahiert nach `lib/log-rotation.js` + 10 neue Unit-Tests (cap, shift, eviction, idempotency, maxBackups=1, invalid input, no-extension)
|
||||||
|
|
||||||
## Open items (priorisiert)
|
## Open items (priorisiert)
|
||||||
|
|
||||||
### Code-Qualität
|
### Code-Qualität
|
||||||
- [ ] **Test-Coverage für 3.3.0** — keine Tests für die queue-cap-prune-Logik in handleBatchDone, sortQueueJobs dynamic-throttle, log-error-recovery.
|
- [ ] queue-cap-prune-Logik (3.3.0 handleBatchDone) — DOM-abhängig, bräuchte jsdom; deferred bis sich anderes lohnt
|
||||||
|
- [ ] sortQueueJobs dynamic-throttle (3.3.0) — modul-state-abhängig im renderer; deferred
|
||||||
|
|
||||||
### UX-Politur
|
### UX-Politur
|
||||||
- [ ] **CSS `.queue-row` transition** auf `:hover` scopen (aktuell auf jedem row → unnötiger Repaint bei Status-Flip).
|
- [ ] **CSS `.queue-row` transition** auf `:hover` scopen (aktuell auf jedem row → unnötiger Repaint bei Status-Flip).
|
||||||
|
|||||||
134
tests/log-rotation.test.js
Normal file
134
tests/log-rotation.test.js
Normal file
@ -0,0 +1,134 @@
|
|||||||
|
const { test, beforeEach, afterEach } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('fs');
|
||||||
|
const path = require('path');
|
||||||
|
const os = require('os');
|
||||||
|
|
||||||
|
const { maybeRotateLogFile } = require('../lib/log-rotation');
|
||||||
|
|
||||||
|
let tmpDir;
|
||||||
|
let logFile;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mhu-log-rotation-'));
|
||||||
|
logFile = path.join(tmpDir, 'fileuploader.log');
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {}
|
||||||
|
});
|
||||||
|
|
||||||
|
function writeBytes(p, n, fill = 'a') {
|
||||||
|
fs.writeFileSync(p, fill.repeat(n), 'utf-8');
|
||||||
|
}
|
||||||
|
|
||||||
|
test('returns false and skips rotation when file does not exist', () => {
|
||||||
|
const result = maybeRotateLogFile(logFile, 100);
|
||||||
|
assert.equal(result, false);
|
||||||
|
assert.equal(fs.existsSync(logFile), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('returns false when file is below the size cap', () => {
|
||||||
|
writeBytes(logFile, 50);
|
||||||
|
const result = maybeRotateLogFile(logFile, 100);
|
||||||
|
assert.equal(result, false);
|
||||||
|
assert.equal(fs.statSync(logFile).size, 50, 'live file untouched');
|
||||||
|
assert.equal(fs.existsSync(logFile + '.1'), false, 'no .1 created');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rotates live file to .1 when over cap', () => {
|
||||||
|
writeBytes(logFile, 200, 'X');
|
||||||
|
const result = maybeRotateLogFile(logFile, 100, 3);
|
||||||
|
assert.equal(result, true);
|
||||||
|
assert.equal(fs.existsSync(logFile), false, 'live file moved away');
|
||||||
|
const expectedBackup = path.join(tmpDir, 'fileuploader.1.log');
|
||||||
|
assert.equal(fs.existsSync(expectedBackup), true, '.1 backup exists');
|
||||||
|
assert.equal(fs.statSync(expectedBackup).size, 200);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('shifts existing backups up: .1 → .2, .2 → .3 on rotation', () => {
|
||||||
|
writeBytes(path.join(tmpDir, 'fileuploader.2.log'), 10, 'B');
|
||||||
|
writeBytes(path.join(tmpDir, 'fileuploader.1.log'), 20, 'A');
|
||||||
|
writeBytes(logFile, 200, 'L');
|
||||||
|
|
||||||
|
const result = maybeRotateLogFile(logFile, 100, 3);
|
||||||
|
assert.equal(result, true);
|
||||||
|
|
||||||
|
// Live file → .1 (latest live data)
|
||||||
|
assert.equal(fs.statSync(path.join(tmpDir, 'fileuploader.1.log')).size, 200);
|
||||||
|
// Old .1 → .2
|
||||||
|
assert.equal(fs.statSync(path.join(tmpDir, 'fileuploader.2.log')).size, 20);
|
||||||
|
// Old .2 → .3
|
||||||
|
assert.equal(fs.statSync(path.join(tmpDir, 'fileuploader.3.log')).size, 10);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('drops oldest backup when at maxBackups limit', () => {
|
||||||
|
// Pre-populate all three backup slots.
|
||||||
|
writeBytes(path.join(tmpDir, 'fileuploader.3.log'), 5, 'C'); // oldest, will be dropped
|
||||||
|
writeBytes(path.join(tmpDir, 'fileuploader.2.log'), 10, 'B');
|
||||||
|
writeBytes(path.join(tmpDir, 'fileuploader.1.log'), 20, 'A');
|
||||||
|
writeBytes(logFile, 200, 'L');
|
||||||
|
|
||||||
|
const result = maybeRotateLogFile(logFile, 100, 3);
|
||||||
|
assert.equal(result, true);
|
||||||
|
|
||||||
|
// Old .3 (5 bytes 'C') gone, replaced by old .2.
|
||||||
|
const f3 = fs.statSync(path.join(tmpDir, 'fileuploader.3.log'));
|
||||||
|
assert.equal(f3.size, 10, 'old .2 became new .3 (the C-file was dropped)');
|
||||||
|
// .2 = old .1
|
||||||
|
assert.equal(fs.statSync(path.join(tmpDir, 'fileuploader.2.log')).size, 20);
|
||||||
|
// .1 = the live file we just rotated
|
||||||
|
assert.equal(fs.statSync(path.join(tmpDir, 'fileuploader.1.log')).size, 200);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('is idempotent — second call on still-large file rotates again', () => {
|
||||||
|
writeBytes(logFile, 200, 'X');
|
||||||
|
maybeRotateLogFile(logFile, 100, 3);
|
||||||
|
// Simulate fresh writes after the first rotation
|
||||||
|
writeBytes(logFile, 200, 'Y');
|
||||||
|
const result = maybeRotateLogFile(logFile, 100, 3);
|
||||||
|
assert.equal(result, true);
|
||||||
|
// The .Y file is now .1, the .X file moved to .2
|
||||||
|
assert.equal(fs.readFileSync(path.join(tmpDir, 'fileuploader.1.log'), 'utf-8')[0], 'Y');
|
||||||
|
assert.equal(fs.readFileSync(path.join(tmpDir, 'fileuploader.2.log'), 'utf-8')[0], 'X');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('maxBackups=1: only keeps a single .1 backup, never .2', () => {
|
||||||
|
writeBytes(logFile, 200, 'L');
|
||||||
|
maybeRotateLogFile(logFile, 100, 1);
|
||||||
|
writeBytes(logFile, 200, 'M');
|
||||||
|
maybeRotateLogFile(logFile, 100, 1);
|
||||||
|
|
||||||
|
// .1 holds the latest rotated content (M)
|
||||||
|
assert.equal(fs.readFileSync(path.join(tmpDir, 'fileuploader.1.log'), 'utf-8')[0], 'M');
|
||||||
|
// .2 must NOT exist
|
||||||
|
assert.equal(fs.existsSync(path.join(tmpDir, 'fileuploader.2.log')), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('invalid maxBytes (0, negative, NaN) is a no-op', () => {
|
||||||
|
writeBytes(logFile, 1000, 'X');
|
||||||
|
for (const max of [0, -1, NaN]) {
|
||||||
|
const r = maybeRotateLogFile(logFile, max);
|
||||||
|
assert.equal(r, false, `maxBytes=${max} should be no-op`);
|
||||||
|
}
|
||||||
|
assert.equal(fs.existsSync(logFile), true);
|
||||||
|
assert.equal(fs.existsSync(logFile + '.1'), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('logs through provided debug callback on rotation', () => {
|
||||||
|
writeBytes(logFile, 200, 'X');
|
||||||
|
const messages = [];
|
||||||
|
maybeRotateLogFile(logFile, 100, 3, (m) => messages.push(m));
|
||||||
|
assert.ok(messages.length >= 1, 'at least one log message');
|
||||||
|
assert.ok(messages.some(m => m.includes('rotated')), `expected "rotated" in: ${messages.join(' | ')}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('handles file without extension correctly', () => {
|
||||||
|
const noExtFile = path.join(tmpDir, 'plainlog');
|
||||||
|
writeBytes(noExtFile, 200, 'P');
|
||||||
|
const result = maybeRotateLogFile(noExtFile, 100, 3);
|
||||||
|
assert.equal(result, true);
|
||||||
|
// base = the full path, ext = '', so backup name is "plainlog.1"
|
||||||
|
assert.equal(fs.existsSync(path.join(tmpDir, 'plainlog.1')), true);
|
||||||
|
assert.equal(fs.existsSync(noExtFile), false);
|
||||||
|
});
|
||||||
Loading…
Reference in New Issue
Block a user