mirror of
https://github.com/browseros-ai/BrowserOS.git
synced 2026-05-20 20:39:10 +00:00
fix: address PR review comments for podman-cpu-memory-env
Claude review on #774 flagged: - readMachineResources/readPositiveInt had no direct test coverage (FakePodmanRuntime overrides initMachine entirely, so the env parsing path was dead code in the test suite). - ensureReady and initMachine both logged 'Initializing Podman machine...' one after the other — two lines for one operation. Fixes: - Export readMachineResources and add 4 unit tests covering defaults, valid parse, non-numeric fallback, and zero/negative fallback. - Reword the initMachine log from a duplicate headline to a concise resource-allocation detail ('Allocating 4 CPUs, 4096 MB RAM') that complements the 'Initializing Podman machine...' headline emitted by ensureReady.
This commit is contained in:
@@ -28,7 +28,7 @@ function readPositiveInt(value: string | undefined, fallback: number): number {
|
||||
return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback
|
||||
}
|
||||
|
||||
function readMachineResources(): { cpus: number; memoryMb: number } {
|
||||
export function readMachineResources(): { cpus: number; memoryMb: number } {
|
||||
return {
|
||||
cpus: readPositiveInt(
|
||||
process.env.BROWSEROS_PODMAN_CPUS,
|
||||
@@ -114,7 +114,7 @@ export class PodmanRuntime {
|
||||
if (isLinux) return
|
||||
|
||||
const { cpus, memoryMb } = readMachineResources()
|
||||
onLog?.(`Initializing Podman machine (${cpus} CPUs, ${memoryMb} MB RAM)`)
|
||||
onLog?.(`Allocating ${cpus} CPUs, ${memoryMb} MB RAM`)
|
||||
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
configurePodmanRuntime,
|
||||
getPodmanRuntime,
|
||||
PodmanRuntime,
|
||||
readMachineResources,
|
||||
resolveBundledPodmanPath,
|
||||
} from '../../../../src/api/services/openclaw/podman-runtime'
|
||||
|
||||
@@ -167,3 +168,46 @@ describe('podman runtime', () => {
|
||||
expect(runtime.startCalls).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('readMachineResources', () => {
|
||||
const ORIGINAL_CPUS = process.env.BROWSEROS_PODMAN_CPUS
|
||||
const ORIGINAL_MEMORY = process.env.BROWSEROS_PODMAN_MEMORY_MB
|
||||
|
||||
afterEach(() => {
|
||||
restoreEnv('BROWSEROS_PODMAN_CPUS', ORIGINAL_CPUS)
|
||||
restoreEnv('BROWSEROS_PODMAN_MEMORY_MB', ORIGINAL_MEMORY)
|
||||
})
|
||||
|
||||
it('returns defaults when env vars are unset', () => {
|
||||
delete process.env.BROWSEROS_PODMAN_CPUS
|
||||
delete process.env.BROWSEROS_PODMAN_MEMORY_MB
|
||||
|
||||
expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 })
|
||||
})
|
||||
|
||||
it('parses valid env values', () => {
|
||||
process.env.BROWSEROS_PODMAN_CPUS = '6'
|
||||
process.env.BROWSEROS_PODMAN_MEMORY_MB = '8192'
|
||||
|
||||
expect(readMachineResources()).toEqual({ cpus: 6, memoryMb: 8192 })
|
||||
})
|
||||
|
||||
it('falls back to defaults for non-numeric input', () => {
|
||||
process.env.BROWSEROS_PODMAN_CPUS = 'abc'
|
||||
process.env.BROWSEROS_PODMAN_MEMORY_MB = ''
|
||||
|
||||
expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 })
|
||||
})
|
||||
|
||||
it('falls back to defaults for zero and negative values', () => {
|
||||
process.env.BROWSEROS_PODMAN_CPUS = '0'
|
||||
process.env.BROWSEROS_PODMAN_MEMORY_MB = '-512'
|
||||
|
||||
expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 })
|
||||
})
|
||||
})
|
||||
|
||||
function restoreEnv(key: string, value: string | undefined): void {
|
||||
if (value === undefined) delete process.env[key]
|
||||
else process.env[key] = value
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user