From 5080b4d647f4b81f2ed407d95636f888fe7d93a7 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 12 Oct 2026 17:31:31 +0100 Subject: [PATCH 1/3] Enforce function length, nullish coalescing and no-console with Biome --- AGENTS.md | 1 + biome.json | 37 ++++- .../modules/azurite/src/azurite-container.ts | 60 ++++--- .../src/cloudstorage-emulator-container.ts | 4 +- .../postgresql/src/postgresql-container.ts | 4 +- .../modules/valkey/src/valkey-container.ts | 2 +- .../src/container-runtime/clients/client.ts | 38 +++-- .../container/docker-container-client.ts | 70 ++++---- .../docker-compose-environment.ts | 157 +++++++++++------- .../generic-container/generic-container.ts | 73 ++++---- .../started-generic-container.ts | 2 +- .../src/wait-strategies/http-wait-strategy.ts | 50 +++--- 12 files changed, 309 insertions(+), 189 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 32afa6fdf..5b7038b04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,7 @@ If a skill or this file turns out to be wrong or incomplete, update it in the sa - Import boundaries are enforced by Biome (`biome.json`): - Modules import core only from the `testcontainers` entry point. The one allowed exception is the core test helper (`../../../testcontainers/src/utils/test-helper`). - `noImportCycles` is an error. Prefer breaking a new cycle (for example with `import type` or a leaf module) over adding a `biome-ignore` suppression. +- Biome also enforces a 50-line limit on non-test functions, `??` over `||` for defaults, and no `console` in package source. Fix a violation rather than suppressing it. ### Dependencies diff --git a/biome.json b/biome.json index 5cd43e809..016d92450 100644 --- a/biome.json +++ b/biome.json @@ -23,11 +23,20 @@ "enabled": true, "rules": { "preset": "recommended", + "complexity": { + "noExcessiveLinesPerFunction": { + "level": "error", + "options": { + "maxLines": 50 + } + } + }, "correctness": { "noUndeclaredDependencies": "error" }, "nursery": { - "noFloatingPromises": "error" + "noFloatingPromises": "error", + "useNullishCoalescing": "error" }, "suspicious": { "noImportCycles": "error" @@ -39,12 +48,38 @@ "includes": ["**/*.test.ts"], "linter": { "rules": { + "complexity": { + "noExcessiveLinesPerFunction": "off" + }, "correctness": { "noUndeclaredDependencies": "off" } } } }, + { + "includes": ["packages/**/src/**", "!packages/testcontainers/src/common/logger.ts"], + "linter": { + "rules": { + "suspicious": { + "noConsole": "error" + } + } + } + }, + { + "includes": ["packages/modules/couchbase/src/couchbase-container.ts"], + "linter": { + "rules": { + "complexity": { + "noExcessiveLinesPerFunction": "off" + }, + "nursery": { + "useNullishCoalescing": "off" + } + } + } + }, { "includes": ["packages/modules/**"], "linter": { diff --git a/packages/modules/azurite/src/azurite-container.ts b/packages/modules/azurite/src/azurite-container.ts index 2a6e98612..d9cc78c83 100755 --- a/packages/modules/azurite/src/azurite-container.ts +++ b/packages/modules/azurite/src/azurite-container.ts @@ -159,6 +159,31 @@ export class AzuriteContainer extends GenericContainer { } public override async start(): Promise { + const command = this.createCommand(); + + this.withCommand(command).withExposedPorts(this.blobPort, this.queuePort, this.tablePort); + + if (this.accountName !== DEFAULT_ACCOUNT_NAME || this.accountKey !== DEFAULT_ACCOUNT_KEY) { + this.withEnvironment({ + AZURITE_ACCOUNTS: `${this.accountName}:${this.accountKey}`, + }); + } + + const protocol: Protocol = this.cert === undefined ? "http" : "https"; + const startedContainer = await super.start(); + + return new StartedAzuriteContainer( + startedContainer, + this.accountName, + this.accountKey, + this.blobPort, + this.queuePort, + this.tablePort, + protocol + ); + } + + private createCommand(): string[] { const command = [ "--blobHost", "0.0.0.0", @@ -190,6 +215,16 @@ export class AzuriteContainer extends GenericContainer { command.push("--skipApiVersionCheck"); } + this.configureSsl(command); + + if (this.oauthEnabled) { + command.push("--oauth", "basic"); + } + + return command; + } + + private configureSsl(command: string[]): void { if (this.cert !== undefined && this.certPath !== undefined) { const contentsToCopy = [ { @@ -216,31 +251,6 @@ export class AzuriteContainer extends GenericContainer { command.push("--pwd", this.password); } } - - if (this.oauthEnabled) { - command.push("--oauth", "basic"); - } - - this.withCommand(command).withExposedPorts(this.blobPort, this.queuePort, this.tablePort); - - if (this.accountName !== DEFAULT_ACCOUNT_NAME || this.accountKey !== DEFAULT_ACCOUNT_KEY) { - this.withEnvironment({ - AZURITE_ACCOUNTS: `${this.accountName}:${this.accountKey}`, - }); - } - - const protocol: Protocol = this.cert === undefined ? "http" : "https"; - const startedContainer = await super.start(); - - return new StartedAzuriteContainer( - startedContainer, - this.accountName, - this.accountKey, - this.blobPort, - this.queuePort, - this.tablePort, - protocol - ); } } diff --git a/packages/modules/gcloud/src/cloudstorage-emulator-container.ts b/packages/modules/gcloud/src/cloudstorage-emulator-container.ts index 9b857b07a..070f2e705 100644 --- a/packages/modules/gcloud/src/cloudstorage-emulator-container.ts +++ b/packages/modules/gcloud/src/cloudstorage-emulator-container.ts @@ -1,5 +1,5 @@ import type { StartedTestContainer } from "testcontainers"; -import { AbstractStartedContainer, GenericContainer, Wait } from "testcontainers"; +import { AbstractStartedContainer, GenericContainer, log, Wait } from "testcontainers"; const PORT = 4443; @@ -101,7 +101,7 @@ export class StartedCloudStorageEmulatorContainer extends AbstractStartedContain }); if (!response.ok) { - console.warn(`error updating fake-gcs-server with external url, response status code: ${response.status}`); + log.warn(`error updating fake-gcs-server with external url, response status code: ${response.status}`); } } diff --git a/packages/modules/postgresql/src/postgresql-container.ts b/packages/modules/postgresql/src/postgresql-container.ts index 9e7281c0f..4e3f7f622 100755 --- a/packages/modules/postgresql/src/postgresql-container.ts +++ b/packages/modules/postgresql/src/postgresql-container.ts @@ -1,4 +1,4 @@ -import { AbstractStartedContainer, GenericContainer, type StartedTestContainer, Wait } from "testcontainers"; +import { AbstractStartedContainer, GenericContainer, log, type StartedTestContainer, Wait } from "testcontainers"; const POSTGRES_PORT = 5432; const POSTGRES_SSL_PATH = "/tmp/testcontainers-node/postgres"; @@ -251,7 +251,7 @@ export class StartedPostgreSqlContainer extends AbstractStartedContainer { throw new Error(`Command failed with exit code ${result.exitCode}: ${result.output}`); } } catch (error) { - console.error(`Failed to execute command: ${command}`, error); + log.error(`Failed to execute command: ${command}: ${error}`); throw error; } } diff --git a/packages/modules/valkey/src/valkey-container.ts b/packages/modules/valkey/src/valkey-container.ts index 92a922a84..913abf0a2 100644 --- a/packages/modules/valkey/src/valkey-container.ts +++ b/packages/modules/valkey/src/valkey-container.ts @@ -77,7 +77,7 @@ export class ValkeyContainer extends GenericContainer { } private async importInitialData(container: StartedTestContainer) { - const re = await container.exec(`/tmp/import.sh ${this.password || ""}`); + const re = await container.exec(`/tmp/import.sh ${this.password ?? ""}`); if (re.exitCode !== 0 || re.output.includes("ERR")) { throw Error(`Could not import initial data from ${this.initialImportScriptFile}: ${re.output}`); } diff --git a/packages/testcontainers/src/container-runtime/clients/client.ts b/packages/testcontainers/src/container-runtime/clients/client.ts index f9a1a8a3a..4821f393d 100644 --- a/packages/testcontainers/src/container-runtime/clients/client.ts +++ b/packages/testcontainers/src/container-runtime/clients/client.ts @@ -17,7 +17,9 @@ import { DockerImageClient } from "./image/docker-image-client"; import type { ImageClient } from "./image/image-client"; import { DockerNetworkClient } from "./network/docker-network-client"; import type { NetworkClient } from "./network/network-client"; -import type { ContainerRuntimeInfo, Info, NodeInfo } from "./types"; +import type { ContainerRuntimeInfo, HostIp, Info, NodeInfo } from "./types"; + +type DockerodeInfo = Awaited>; export class ContainerRuntimeClient { constructor( @@ -94,12 +96,6 @@ async function initStrategy(strategy: ContainerRuntimeClientStrategy): Promise): Promise { - const execOptions: ExecCreateOptions = { - Cmd: command, - AttachStdout: true, - AttachStderr: true, - }; - - if (opts?.env !== undefined) { - execOptions.Env = Object.entries(opts.env).map(([key, value]) => `${key}=${value}`); - } - if (opts?.workingDir !== undefined) { - execOptions.WorkingDir = opts.workingDir; - } - if (opts?.user !== undefined) { - execOptions.User = opts.user; - } + const execOptions = this.createExecOptions(command, opts); const outputChunks: string[] = []; const stdoutChunks: string[] = []; @@ -259,18 +245,7 @@ export class DockerContainerClient implements ContainerClient { processStream(stdoutStream, stdoutChunks); processStream(stderrStream, stderrChunks); - try { - await new Promise((res, rej) => { - stream.on("end", res); - stream.on("error", rej); - }); - - stdoutStream.end(); - stderrStream.end(); - await Promise.all([finished(stdoutStream), finished(stderrStream)]); - } finally { - stream.destroy(); - } + await this.waitForExecStreamToEnd(stream, stdoutStream, stderrStream); const inspectResult = await exec.inspect(); const exitCode = inspectResult.ExitCode ?? -1; @@ -290,6 +265,45 @@ export class DockerContainerClient implements ContainerClient { } } + private createExecOptions(command: string[], opts?: Partial): ExecCreateOptions { + const execOptions: ExecCreateOptions = { + Cmd: command, + AttachStdout: true, + AttachStderr: true, + }; + + if (opts?.env !== undefined) { + execOptions.Env = Object.entries(opts.env).map(([key, value]) => `${key}=${value}`); + } + if (opts?.workingDir !== undefined) { + execOptions.WorkingDir = opts.workingDir; + } + if (opts?.user !== undefined) { + execOptions.User = opts.user; + } + + return execOptions; + } + + private async waitForExecStreamToEnd( + stream: Duplex, + stdoutStream: PassThrough, + stderrStream: PassThrough + ): Promise { + try { + await new Promise((res, rej) => { + stream.on("end", res); + stream.on("error", rej); + }); + + stdoutStream.end(); + stderrStream.end(); + await Promise.all([finished(stdoutStream), finished(stderrStream)]); + } finally { + stream.destroy(); + } + } + async restart(container: Container, opts?: { timeout: number }): Promise { try { log.debug(`Restarting container...`, { containerId: container.id }); diff --git a/packages/testcontainers/src/docker-compose-environment/docker-compose-environment.ts b/packages/testcontainers/src/docker-compose-environment/docker-compose-environment.ts index 2d22e3f89..c1356bda4 100644 --- a/packages/testcontainers/src/docker-compose-environment/docker-compose-environment.ts +++ b/packages/testcontainers/src/docker-compose-environment/docker-compose-environment.ts @@ -1,6 +1,11 @@ import type { ContainerInfo } from "dockerode"; import { containerLog, log, RandomUuid, type Uuid } from "../common"; -import { type ComposeOptions, getContainerRuntimeClient, parseComposeContainerName } from "../container-runtime"; +import { + type ComposeOptions, + type ContainerRuntimeClient, + getContainerRuntimeClient, + parseComposeContainerName, +} from "../container-runtime"; import { StartedGenericContainer } from "../generic-container/started-generic-container"; import { getReaper } from "../reaper/reaper"; import type { Environment } from "../types"; @@ -119,12 +124,40 @@ export class DockerComposeEnvironment { projectName: this.projectName, }; + const shouldPull = this.pullPolicy.shouldPull(); + const commandOptions = this.createCommandOptions(clientCommandOptions, shouldPull); + const composeOptions = this.createComposeOptions(clientComposeOptions); + + if (shouldPull === true) { + await client.compose.pull(options, services); + } + await client.compose.up( + { + ...options, + commandOptions, + composeOptions, + environment: { ...this.environment }, + }, + services + ); + + const startedGenericContainers = await this.waitForStartedContainers(client, options); + + log.info(`DockerCompose environment started`); + + return new StartedDockerComposeEnvironment(startedGenericContainers, { + ...options, + composeOptions, + environment: this.environment, + }); + } + + private createCommandOptions(clientCommandOptions: string[], shouldPull: boolean | "never"): string[] { const commandOptions = [...clientCommandOptions]; if (this.build) { commandOptions.push("--build"); } - const shouldPull = this.pullPolicy.shouldPull(); if (shouldPull === "never") { commandOptions.push("--pull", "never"); } @@ -132,6 +165,10 @@ export class DockerComposeEnvironment { commandOptions.push("--no-recreate"); } + return commandOptions; + } + + private createComposeOptions(clientComposeOptions: string[]): string[] { const composeOptions = [...clientComposeOptions]; if (this.environmentFile) { composeOptions.push("--env-file", this.environmentFile); @@ -140,19 +177,13 @@ export class DockerComposeEnvironment { composeOptions.push("--profile", profile); }); - if (shouldPull === true) { - await client.compose.pull(options, services); - } - await client.compose.up( - { - ...options, - commandOptions, - composeOptions, - environment: { ...this.environment }, - }, - services - ); + return composeOptions; + } + private async waitForStartedContainers( + client: ContainerRuntimeClient, + options: ComposeOptions + ): Promise<{ [containerName: string]: StartedGenericContainer }> { const startedContainers = (await client.container.list()).filter( (container) => container.Labels["com.docker.compose.project"] === this.projectName ); @@ -168,51 +199,9 @@ export class DockerComposeEnvironment { ); this.warnForUnusedWaitStrategies(startedContainerNameSet); - const startedGenericContainers = ( + return ( await Promise.all( - startedContainers.map(async (startedContainer) => { - const container = client.container.getById(startedContainer.Id); - const containerName = parseComposeContainerName(this.projectName, startedContainer.Names[0]); - - const inspectResult = await client.container.inspect(container); - const mappedInspectResult = mapInspectResult(inspectResult); - const boundPorts = BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, mappedInspectResult); - const waitStrategy = await selectWaitStrategy({ - client, - inspectResult, - waitStrategy: this.waitStrategy[containerName] ?? this.defaultWaitStrategy, - }); - if (this.startupTimeoutMs !== undefined) { - waitStrategy.withStartupTimeout(this.startupTimeoutMs); - } - - if (containerLog.enabled()) { - (await client.container.logs(container)) - .on("data", (data) => containerLog.trace(`${containerName}: ${data.trim()}`)) - .on("err", (data) => containerLog.error(`${containerName}: ${data.trim()}`)); - } - - try { - await waitForContainer(client, container, waitStrategy, boundPorts); - } catch (err) { - try { - await client.compose.down(options, { removeVolumes: true, timeout: 0 }); - } catch { - log.warn(`Failed to stop DockerCompose environment after failed up`); - } - throw err; - } - - return new StartedGenericContainer( - container, - client.info.containerRuntime.host, - inspectResult, - boundPorts, - containerName, - waitStrategy, - true - ); - }) + startedContainers.map((startedContainer) => this.toStartedGenericContainer(client, options, startedContainer)) ) ).reduce<{ [containerName: string]: StartedGenericContainer }>((map, startedGenericContainer) => { // defineProperty rather than assignment: a container named "__proto__" would otherwise @@ -225,14 +214,54 @@ export class DockerComposeEnvironment { }); return map; }, {}); + } - log.info(`DockerCompose environment started`); - - return new StartedDockerComposeEnvironment(startedGenericContainers, { - ...options, - composeOptions, - environment: this.environment, + private async toStartedGenericContainer( + client: ContainerRuntimeClient, + options: ComposeOptions, + startedContainer: ContainerInfo + ): Promise { + const container = client.container.getById(startedContainer.Id); + const containerName = parseComposeContainerName(this.projectName, startedContainer.Names[0]); + + const inspectResult = await client.container.inspect(container); + const mappedInspectResult = mapInspectResult(inspectResult); + const boundPorts = BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, mappedInspectResult); + const waitStrategy = await selectWaitStrategy({ + client, + inspectResult, + waitStrategy: this.waitStrategy[containerName] ?? this.defaultWaitStrategy, }); + if (this.startupTimeoutMs !== undefined) { + waitStrategy.withStartupTimeout(this.startupTimeoutMs); + } + + if (containerLog.enabled()) { + (await client.container.logs(container)) + .on("data", (data) => containerLog.trace(`${containerName}: ${data.trim()}`)) + .on("err", (data) => containerLog.error(`${containerName}: ${data.trim()}`)); + } + + try { + await waitForContainer(client, container, waitStrategy, boundPorts); + } catch (err) { + try { + await client.compose.down(options, { removeVolumes: true, timeout: 0 }); + } catch { + log.warn(`Failed to stop DockerCompose environment after failed up`); + } + throw err; + } + + return new StartedGenericContainer( + container, + client.info.containerRuntime.host, + inspectResult, + boundPorts, + containerName, + waitStrategy, + true + ); } private warnForUnusedWaitStrategies(startedContainerNames: Set): void { diff --git a/packages/testcontainers/src/generic-container/generic-container.ts b/packages/testcontainers/src/generic-container/generic-container.ts index d9b2f8ffd..c38bcac44 100644 --- a/packages/testcontainers/src/generic-container/generic-container.ts +++ b/packages/testcontainers/src/generic-container/generic-container.ts @@ -199,26 +199,8 @@ export class GenericContainer implements TestContainer { private async startContainer(client: ContainerRuntimeClient): Promise { const container = await client.container.create({ ...this.createOpts, HostConfig: this.hostConfig }); - if (!this.isHelperContainer() && PortForwarderInstance.isRunning()) { - await this.connectContainerToPortForwarder(client, container); - } - - if (this.networkMode && this.networkAliases.length > 0) { - const network = client.network.getById(this.networkMode); - await client.container.connectToNetwork(container, network, this.networkAliases); - } - - if (this.filesToCopy.length > 0 || this.directoriesToCopy.length > 0 || this.contentsToCopy.length > 0) { - const archive = await this.createArchiveToCopyToContainer(); - await Promise.all([ - archive.finalize(), - client.container.putArchive(container, archive, "/", this.copyToContainerOptions), - ]); - } - - for (const archive of this.archivesToCopy) { - await client.container.putArchive(container, archive.tar, archive.target, this.copyToContainerOptions); - } + await this.connectContainerToNetworks(client, container); + await this.copyToCreatedContainer(client, container); log.info(`Starting container for image "${this.createOpts.Image}"...`, { containerId: container.id }); if (this.containerCreated) { @@ -237,17 +219,7 @@ export class GenericContainer implements TestContainer { this.exposedPorts ); - if (containerLog.enabled() || this.logConsumer !== undefined) { - if (this.logConsumer !== undefined) { - this.logConsumer(await client.container.logs(container)); - } - - if (containerLog.enabled()) { - (await client.container.logs(container)) - .on("data", (data) => containerLog.trace(data.trim(), { containerId: container.id })) - .on("err", (data) => containerLog.error(data.trim(), { containerId: container.id })); - } - } + await this.consumeContainerLogs(client, container); if (this.containerStarting) { await this.containerStarting(mappedInspectResult, false); @@ -277,6 +249,17 @@ export class GenericContainer implements TestContainer { return startedContainer; } + private async connectContainerToNetworks(client: ContainerRuntimeClient, container: Container): Promise { + if (!this.isHelperContainer() && PortForwarderInstance.isRunning()) { + await this.connectContainerToPortForwarder(client, container); + } + + if (this.networkMode && this.networkAliases.length > 0) { + const network = client.network.getById(this.networkMode); + await client.container.connectToNetwork(container, network, this.networkAliases); + } + } + private async connectContainerToPortForwarder(client: ContainerRuntimeClient, container: Container) { const portForwarder = await PortForwarderInstance.getInstance(); const portForwarderNetworkId = portForwarder.getNetworkId(); @@ -288,6 +271,20 @@ export class GenericContainer implements TestContainer { } } + private async copyToCreatedContainer(client: ContainerRuntimeClient, container: Container): Promise { + if (this.filesToCopy.length > 0 || this.directoriesToCopy.length > 0 || this.contentsToCopy.length > 0) { + const archive = await this.createArchiveToCopyToContainer(); + await Promise.all([ + archive.finalize(), + client.container.putArchive(container, archive, "/", this.copyToContainerOptions), + ]); + } + + for (const archive of this.archivesToCopy) { + await client.container.putArchive(container, archive.tar, archive.target, this.copyToContainerOptions); + } + } + private async createArchiveToCopyToContainer(): Promise { const tar = archiver("tar"); const filesToCopyWithStats = await Promise.all( @@ -310,6 +307,20 @@ export class GenericContainer implements TestContainer { return tar; } + private async consumeContainerLogs(client: ContainerRuntimeClient, container: Container): Promise { + if (containerLog.enabled() || this.logConsumer !== undefined) { + if (this.logConsumer !== undefined) { + this.logConsumer(await client.container.logs(container)); + } + + if (containerLog.enabled()) { + (await client.container.logs(container)) + .on("data", (data) => containerLog.trace(data.trim(), { containerId: container.id })) + .on("err", (data) => containerLog.error(data.trim(), { containerId: container.id })); + } + } + } + protected containerStarted?( container: StartedTestContainer, inspectResult: InspectResult, diff --git a/packages/testcontainers/src/generic-container/started-generic-container.ts b/packages/testcontainers/src/generic-container/started-generic-container.ts index fa23559ad..6539caf40 100644 --- a/packages/testcontainers/src/generic-container/started-generic-container.ts +++ b/packages/testcontainers/src/generic-container/started-generic-container.ts @@ -63,7 +63,7 @@ export class StartedGenericContainer implements StartedTestContainer { client: ContainerRuntimeClient; }): Promise { const { deleteOnExit, client } = options; - const changes = options.changes || []; + const changes = options.changes ?? []; if (deleteOnExit) { let sessionId = this.getLabels()[LABEL_TESTCONTAINERS_SESSION_ID]; if (!sessionId) { diff --git a/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts index b707ba564..a4eecc633 100644 --- a/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts @@ -100,14 +100,7 @@ export class HttpWaitStrategy extends AbstractWaitStrategy { } } - return undiciResponseToFetchResponse( - await request(url, { - method: this.method, - signal: AbortSignal.timeout(this.readTimeoutMs), - headers: this.headers, - dispatcher: agent, - }) - ); + return await this.sendRequest(url, agent); } catch { return undefined; } @@ -117,19 +110,7 @@ export class HttpWaitStrategy extends AbstractWaitStrategy { return true; } - if (response === undefined) { - return false; - } else if (!this.predicates.length) { - return response.ok; - } else { - for (const predicate of this.predicates) { - const result = await predicate(response); - if (!result) { - return false; - } - } - return true; - } + return this.matchesPredicates(response); }, () => { const message = `URL ${this.path} not accessible after ${this.startupTimeoutMs}ms`; @@ -151,6 +132,33 @@ export class HttpWaitStrategy extends AbstractWaitStrategy { log.debug(`HTTP wait strategy complete`, { containerId: container.id }); } + private async sendRequest(url: string, agent: Agent | undefined): Promise { + return undiciResponseToFetchResponse( + await request(url, { + method: this.method, + signal: AbortSignal.timeout(this.readTimeoutMs), + headers: this.headers, + dispatcher: agent, + }) + ); + } + + private async matchesPredicates(response: Response | undefined): Promise { + if (response === undefined) { + return false; + } else if (!this.predicates.length) { + return response.ok; + } else { + for (const predicate of this.predicates) { + const result = await predicate(response); + if (!result) { + return false; + } + } + return true; + } + } + private async handleContainerExit(container: Dockerode.Container) { const tail = 50; const lastLogs: string[] = []; From a54d4f08fa55a61ecf0104339755d7cd7fc5afa1 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 12 Oct 2026 19:27:35 +0100 Subject: [PATCH 2/3] Remove the Couchbase override and split createBuckets --- biome.json | 13 - .../couchbase/src/couchbase-container.ts | 358 ++++++++++-------- 2 files changed, 200 insertions(+), 171 deletions(-) diff --git a/biome.json b/biome.json index 016d92450..44bb21126 100644 --- a/biome.json +++ b/biome.json @@ -67,19 +67,6 @@ } } }, - { - "includes": ["packages/modules/couchbase/src/couchbase-container.ts"], - "linter": { - "rules": { - "complexity": { - "noExcessiveLinesPerFunction": "off" - }, - "nursery": { - "useNullishCoalescing": "off" - } - } - } - }, { "includes": ["packages/modules/**"], "linter": { diff --git a/packages/modules/couchbase/src/couchbase-container.ts b/packages/modules/couchbase/src/couchbase-container.ts index 6b1232f58..fff37c978 100644 --- a/packages/modules/couchbase/src/couchbase-container.ts +++ b/packages/modules/couchbase/src/couchbase-container.ts @@ -387,121 +387,131 @@ export class CouchbaseContainer extends GenericContainer { for (const bucket of this.buckets) { log.debug(`Creating bucket ${bucket.getName()}`); - const body = new URLSearchParams(); - body.set("name", bucket.getName()); - body.set("ramQuotaMB", bucket.getQuota().toString()); - body.set("flushEnabled", bucket.hasFlushEnabled() ? "1" : "0"); - body.set("replicaNumber", bucket.getNumOfReplicas().toString()); - - const response = await this.doHttpRequest( - startedTestContainer.getHost(), - startedTestContainer.getMappedPort(PORTS.MGMT_PORT), - "/pools/default/buckets", - "POST", - body, - true + await this.createBucket(startedTestContainer, bucket); + await this.waitUntilBucketHasEnabledServices( + client, + inspectResult, + startedTestContainer, + bucket, + remainingStartupMs() ); - - await this.checkResponse(response, `Could not create bucket ${bucket.getName()}`); - - await Wait.forHttp(`/pools/default/b/${bucket.getName()}`, PORTS.MGMT_PORT) - .withBasicCredentials(this.username, this.password) - .forStatusCode(200) - .withStartupTimeout(remainingStartupMs()) - .forResponsePredicate((response) => { - try { - const jsonResponse = JSON.parse(response); - const services = jsonResponse.nodesExt[0].services; - const serviceNames = Object.keys(services); - - let found = false; - for (const enabledService of this.enabledServices) { - if (serviceNames.some((s) => s.startsWith(enabledService.getIdentifier()))) { - found = true; - } else { - found = false; - log.trace(`Service ${enabledService.getIdentifier()} not yet part of config, retrying.`); - break; - } - } - return found; - } catch (e) { - log.error(`Unable to parse response: ${response} ${e}`); - return false; - } - }) - .waitUntilReady( - client.container.getById(startedTestContainer.getId()), - BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult) - ); - - await Wait.forHttp(`/pools/default/buckets/${bucket.getName()}`, PORTS.MGMT_PORT) - .withBasicCredentials(this.username, this.password) - .forStatusCode(200) - .withStartupTimeout(remainingStartupMs()) - .forResponsePredicate((response) => { - try { - const jsonResponse = JSON.parse(response); - return jsonResponse.nodes[0]?.status === "healthy"; - } catch (e) { - log.error(`Unable to parse response: ${response}, error: ${e}`); - return false; - } - }) - .waitUntilReady( - client.container.getById(startedTestContainer.getId()), - BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult) - ); + await this.waitUntilBucketIsHealthy(client, inspectResult, startedTestContainer, bucket, remainingStartupMs()); if (this.enabledServices.has(CouchbaseService.QUERY)) { - const timeoutMs = remainingStartupMs(); - - await new IntervalRetry(1000).retryUntil( - async () => { - try { - const body = new URLSearchParams(); - body.set( - "statement", - `SELECT COUNT(*) > 0 as present FROM system:keyspaces WHERE name = "${bucket.getName()}"` - ); - - const response = await this.doHttpRequest( - startedTestContainer.getHost(), - startedTestContainer.getMappedPort(PORTS.QUERY_PORT), - "/query/service", - "POST", - body, - true - ); - - await this.checkResponse(response, `Could not poll query service state for bucket: ${bucket.getName()}`); - - return response; - } catch (_e) { - return undefined; - } - }, - async (response) => { - if (response === undefined) { - return false; - } - const jsonResponse = (await response.json()) as { results: Array<{ present: boolean }> }; - return jsonResponse.results[0].present; - }, - () => { - const message = `URL /query/service not accessible after ${timeoutMs}ms`; - log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id }); - - throw new Error(message); - }, - timeoutMs - ); + await this.waitUntilKeyspaceIsPresent(client, startedTestContainer, bucket, remainingStartupMs()); } if (bucket.hasPrimaryIndex()) { if (this.enabledServices.has(CouchbaseService.QUERY)) { + await this.createPrimaryIndex(startedTestContainer, bucket); + await this.waitUntilPrimaryIndexIsOnline(client, startedTestContainer, bucket, remainingStartupMs()); + } else { + log.info( + `Primary index creation for bucket ${bucket.getName()} ignored, since QUERY service is not present.` + ); + } + } + } + } + + private async createBucket(startedTestContainer: StartedTestContainer, bucket: BucketDefinition) { + const body = new URLSearchParams(); + body.set("name", bucket.getName()); + body.set("ramQuotaMB", bucket.getQuota().toString()); + body.set("flushEnabled", bucket.hasFlushEnabled() ? "1" : "0"); + body.set("replicaNumber", bucket.getNumOfReplicas().toString()); + + const response = await this.doHttpRequest( + startedTestContainer.getHost(), + startedTestContainer.getMappedPort(PORTS.MGMT_PORT), + "/pools/default/buckets", + "POST", + body, + true + ); + + await this.checkResponse(response, `Could not create bucket ${bucket.getName()}`); + } + + private async waitUntilBucketHasEnabledServices( + client: ContainerRuntimeClient, + inspectResult: InspectResult, + startedTestContainer: StartedTestContainer, + bucket: BucketDefinition, + timeoutMs: number + ) { + await Wait.forHttp(`/pools/default/b/${bucket.getName()}`, PORTS.MGMT_PORT) + .withBasicCredentials(this.username, this.password) + .forStatusCode(200) + .withStartupTimeout(timeoutMs) + .forResponsePredicate((response) => { + try { + const jsonResponse = JSON.parse(response); + const services = jsonResponse.nodesExt[0].services; + const serviceNames = Object.keys(services); + + let found = false; + for (const enabledService of this.enabledServices) { + if (serviceNames.some((s) => s.startsWith(enabledService.getIdentifier()))) { + found = true; + } else { + found = false; + log.trace(`Service ${enabledService.getIdentifier()} not yet part of config, retrying.`); + break; + } + } + return found; + } catch (e) { + log.error(`Unable to parse response: ${response} ${e}`); + return false; + } + }) + .waitUntilReady( + client.container.getById(startedTestContainer.getId()), + BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult) + ); + } + + private async waitUntilBucketIsHealthy( + client: ContainerRuntimeClient, + inspectResult: InspectResult, + startedTestContainer: StartedTestContainer, + bucket: BucketDefinition, + timeoutMs: number + ) { + await Wait.forHttp(`/pools/default/buckets/${bucket.getName()}`, PORTS.MGMT_PORT) + .withBasicCredentials(this.username, this.password) + .forStatusCode(200) + .withStartupTimeout(timeoutMs) + .forResponsePredicate((response) => { + try { + const jsonResponse = JSON.parse(response); + return jsonResponse.nodes[0]?.status === "healthy"; + } catch (e) { + log.error(`Unable to parse response: ${response}, error: ${e}`); + return false; + } + }) + .waitUntilReady( + client.container.getById(startedTestContainer.getId()), + BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult) + ); + } + + private async waitUntilKeyspaceIsPresent( + client: ContainerRuntimeClient, + startedTestContainer: StartedTestContainer, + bucket: BucketDefinition, + timeoutMs: number + ) { + await new IntervalRetry(1000).retryUntil( + async () => { + try { const body = new URLSearchParams(); - body.set("statement", `CREATE PRIMARY INDEX on \`${bucket.getName()}\``); //TODO: check here! + body.set( + "statement", + `SELECT COUNT(*) > 0 as present FROM system:keyspaces WHERE name = "${bucket.getName()}"` + ); const response = await this.doHttpRequest( startedTestContainer.getHost(), @@ -512,60 +522,92 @@ export class CouchbaseContainer extends GenericContainer { true ); - await this.checkResponse(response, `Could not create primary index for bucket ${bucket.getName()}`); - - const timeoutMs = remainingStartupMs(); - - await new IntervalRetry(1000).retryUntil( - async () => { - try { - const body = new URLSearchParams(); - body.set( - "statement", - `SELECT COUNT(*) > 0 as online FROM system:indexes WHERE keyspace_id = "${bucket.getName()}" and is_primary = true and state = "online"` - ); - - const response = await this.doHttpRequest( - startedTestContainer.getHost(), - startedTestContainer.getMappedPort(PORTS.QUERY_PORT), - "/query/service", - "POST", - body, - true - ); - - await this.checkResponse( - response, - `Could not poll primary index state for bucket: ${bucket.getName()}` - ); - - return response; - } catch (_e) { - return undefined; - } - }, - async (response) => { - if (response === undefined) { - return false; - } - const jsonResponse = (await response.json()) as { results: Array<{ online: boolean }> }; - return jsonResponse.results[0].online; - }, - () => { - const message = `URL /query/service not accessible after ${timeoutMs}ms`; - log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id }); - - throw new Error(message); - }, - timeoutMs + await this.checkResponse(response, `Could not poll query service state for bucket: ${bucket.getName()}`); + + return response; + } catch (_e) { + return undefined; + } + }, + async (response) => { + if (response === undefined) { + return false; + } + const jsonResponse = (await response.json()) as { results: Array<{ present: boolean }> }; + return jsonResponse.results[0].present; + }, + () => { + const message = `URL /query/service not accessible after ${timeoutMs}ms`; + log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id }); + + throw new Error(message); + }, + timeoutMs + ); + } + + private async createPrimaryIndex(startedTestContainer: StartedTestContainer, bucket: BucketDefinition) { + const body = new URLSearchParams(); + body.set("statement", `CREATE PRIMARY INDEX on \`${bucket.getName()}\``); //TODO: check here! + + const response = await this.doHttpRequest( + startedTestContainer.getHost(), + startedTestContainer.getMappedPort(PORTS.QUERY_PORT), + "/query/service", + "POST", + body, + true + ); + + await this.checkResponse(response, `Could not create primary index for bucket ${bucket.getName()}`); + } + + private async waitUntilPrimaryIndexIsOnline( + client: ContainerRuntimeClient, + startedTestContainer: StartedTestContainer, + bucket: BucketDefinition, + timeoutMs: number + ) { + await new IntervalRetry(1000).retryUntil( + async () => { + try { + const body = new URLSearchParams(); + body.set( + "statement", + `SELECT COUNT(*) > 0 as online FROM system:indexes WHERE keyspace_id = "${bucket.getName()}" and is_primary = true and state = "online"` ); - } else { - log.info( - `Primary index creation for bucket ${bucket.getName()} ignored, since QUERY service is not present.` + + const response = await this.doHttpRequest( + startedTestContainer.getHost(), + startedTestContainer.getMappedPort(PORTS.QUERY_PORT), + "/query/service", + "POST", + body, + true ); + + await this.checkResponse(response, `Could not poll primary index state for bucket: ${bucket.getName()}`); + + return response; + } catch (_e) { + return undefined; } - } - } + }, + async (response) => { + if (response === undefined) { + return false; + } + const jsonResponse = (await response.json()) as { results: Array<{ online: boolean }> }; + return jsonResponse.results[0].online; + }, + () => { + const message = `URL /query/service not accessible after ${timeoutMs}ms`; + log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id }); + + throw new Error(message); + }, + timeoutMs + ); } protected override async containerStarted( From f31b1bc4a0da7604fb43cb4638ae613dbb806754 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 12 Oct 2026 19:39:25 +0100 Subject: [PATCH 3/3] Replace the Biome rule list in AGENTS.md with one line The rules live in biome.json. AGENTS.md keeps only the guidance that config can't express: fix a violation rather than suppressing it. --- AGENTS.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7ad10adb4..43d0826b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,10 +35,7 @@ If a skill or this file turns out to be wrong or incomplete, update it in the sa - Fix the root cause. A change that only makes a failure less likely is not a fix. - Arbitrary delays (`sleep`), repeating a check, or adding retries do not remove a race; they hide it. - For readiness problems, wait on a deterministic signal of the state you need. If no such signal exists, say so rather than shipping a mitigation as a fix. -- Import boundaries are enforced by Biome (`biome.json`): - - Modules import core only from the `testcontainers` entry point. The one allowed exception is the core test helper (`../../../testcontainers/src/utils/test-helper`). - - `noImportCycles` is an error. Prefer breaking a new cycle (for example with `import type` or a leaf module) over adding a `biome-ignore` suppression. -- Biome also enforces a 50-line limit on non-test functions, `??` over `||` for defaults, and no `console` in package source. Fix a violation rather than suppressing it. +- Fix a lint violation rather than suppressing it with `biome-ignore`. ### Dependencies