From 81391324651974b8f000a972349511d496a0efb0 Mon Sep 17 00:00:00 2001 From: Peter Somogyvari Date: Fri, 1 Mar 2024 10:54:34 -0800 Subject: [PATCH] refactor(test-tooling): fix types of streams: use NodeJS.ReadableStream 1. The container management library that we use in the test infrastructure (called dockerode) is expecting streams that are defined in the global namespace of the `@types/node` library, e.g. the standard library of NodeJS itself. 2. Previously we were using the "streams" package to provide type information to the streams that we were passing around to dockerode and it was working fine, but after some changes that seem unrelated this has broken the compilation process. 3. The mentioned changes are not yet on the main branch, but we expect them to be there soon and so this change is laying the groundwork for that by pre-emptively fixing the broken build's root cause which is that the test-tooling package does not declare it's typings related dependencies correctly: It implicitly uses the NodeJS standard library's types but so far had not declared them on the package level. 4. This change is therefore to rectify the issue of the `@types/node` dependency missing from the test-tooling package and also the refactoring of some of the test ledger classes which were relying on the `streams` builtin package instead of correctly using the NodeJS.ReadableStream global. 5. Earlier the reasoning for this was that we try to avoid pulling in types from the global scope because we try to avoid any sort of dependency on the global scope in general. Once we have proof though that this is causing issues with the build, then we must give up the principle for practical reasons (and only in the minimum viable scope, e.g. this does not change the fact that everywhere else in the codebase we should still do our best to avoid using the global scoped classes, types, functions, etc..). Thank you to @AndreAugusto11 and @RafaelAPB for pointing out this issue through the pull request of his that is currently being worked on at the time of this writing: https://github.com/RafaelAPB/blockchain-integration-framework/pull/72 Related to but does not address #2811 Signed-off-by: Peter Somogyvari --- packages/cactus-test-tooling/package.json | 1 + .../src/main/typescript/common/containers.ts | 7 ++-- .../http-echo/http-echo-container.ts | 36 ++++++++++--------- .../postgres/postgres-test-container.ts | 36 ++++++++++--------- .../typescript/quorum/quorum-test-ledger.ts | 36 ++++++++++--------- yarn.lock | 10 ++++++ 6 files changed, 73 insertions(+), 53 deletions(-) diff --git a/packages/cactus-test-tooling/package.json b/packages/cactus-test-tooling/package.json index 813b426cadc..e4993e7fd37 100644 --- a/packages/cactus-test-tooling/package.json +++ b/packages/cactus-test-tooling/package.json @@ -101,6 +101,7 @@ "@types/fs-extra": "9.0.13", "@types/js-yaml": "4.0.3", "@types/lodash": "4.14.172", + "@types/node": "20.11.24", "@types/node-forge": "1.3.0", "@types/ssh2": "0.5.47", "@types/ssh2-streams": "0.1.9", diff --git a/packages/cactus-test-tooling/src/main/typescript/common/containers.ts b/packages/cactus-test-tooling/src/main/typescript/common/containers.ts index fbedd731ef3..4b575e9a4be 100644 --- a/packages/cactus-test-tooling/src/main/typescript/common/containers.ts +++ b/packages/cactus-test-tooling/src/main/typescript/common/containers.ts @@ -1,5 +1,5 @@ import path from "path"; -import { Duplex, Stream } from "stream"; +import { Duplex } from "stream"; import { IncomingMessage } from "http"; import throttle from "lodash/throttle"; import { Container, ContainerInfo } from "dockerode"; @@ -443,7 +443,10 @@ export class Containers { log.debug(JSON.stringify(msg.progress || msg.status)); }, 1000); - const pullStreamStartedHandler = (pullError: unknown, stream: Stream) => { + const pullStreamStartedHandler = ( + pullError: unknown, + stream: NodeJS.ReadableStream, + ) => { if (pullError) { log.error(`Could not even start ${imageFqn} pull:`, pullError); reject(pullError); diff --git a/packages/cactus-test-tooling/src/main/typescript/http-echo/http-echo-container.ts b/packages/cactus-test-tooling/src/main/typescript/http-echo/http-echo-container.ts index 75723d93044..938fe4445c3 100644 --- a/packages/cactus-test-tooling/src/main/typescript/http-echo/http-echo-container.ts +++ b/packages/cactus-test-tooling/src/main/typescript/http-echo/http-echo-container.ts @@ -3,7 +3,6 @@ import isPortReachable from "is-port-reachable"; import Joi from "joi"; import { EventEmitter } from "events"; import { ITestLedger } from "../i-test-ledger"; -import { Stream } from "stream"; const OPTS_SCHEMA: Joi.Schema = Joi.object().keys({ imageVersion: Joi.string().min(5).required(), @@ -211,22 +210,25 @@ export class HttpEchoContainer implements ITestLedger { private pullContainerImage(containerNameAndTag: string): Promise { return new Promise((resolve, reject) => { const docker = new Docker(); - docker.pull(containerNameAndTag, (pullError: unknown, stream: Stream) => { - if (pullError) { - reject(pullError); - } else { - docker.modem.followProgress( - stream, - (progressError: unknown, output: unknown[]) => { - if (progressError) { - reject(progressError); - } else { - resolve(output); - } - }, - ); - } - }); + docker.pull( + containerNameAndTag, + (pullError: unknown, stream: NodeJS.ReadableStream) => { + if (pullError) { + reject(pullError); + } else { + docker.modem.followProgress( + stream, + (progressError: unknown, output: unknown[]) => { + if (progressError) { + reject(progressError); + } else { + resolve(output); + } + }, + ); + } + }, + ); }); } diff --git a/packages/cactus-test-tooling/src/main/typescript/postgres/postgres-test-container.ts b/packages/cactus-test-tooling/src/main/typescript/postgres/postgres-test-container.ts index 9a203ba8e7e..95885132e24 100644 --- a/packages/cactus-test-tooling/src/main/typescript/postgres/postgres-test-container.ts +++ b/packages/cactus-test-tooling/src/main/typescript/postgres/postgres-test-container.ts @@ -11,7 +11,6 @@ import { import { ITestLedger } from "../i-test-ledger"; import { Streams } from "../common/streams"; import { Containers } from "../common/containers"; -import { Stream } from "stream"; /* * Contains options for Postgres container @@ -287,22 +286,25 @@ export class PostgresTestContainer implements ITestLedger { private pullContainerImage(containerNameAndTag: string): Promise { return new Promise((resolve, reject) => { const docker = new Docker(); - docker.pull(containerNameAndTag, (pullError: unknown, stream: Stream) => { - if (pullError) { - reject(pullError); - } else { - docker.modem.followProgress( - stream, - (progressError: unknown, output: unknown[]) => { - if (progressError) { - reject(progressError); - } else { - resolve(output); - } - }, - ); - } - }); + docker.pull( + containerNameAndTag, + (pullError: unknown, stream: NodeJS.ReadableStream) => { + if (pullError) { + reject(pullError); + } else { + docker.modem.followProgress( + stream, + (progressError: unknown, output: unknown[]) => { + if (progressError) { + reject(progressError); + } else { + resolve(output); + } + }, + ); + } + }, + ); }); } diff --git a/packages/cactus-test-tooling/src/main/typescript/quorum/quorum-test-ledger.ts b/packages/cactus-test-tooling/src/main/typescript/quorum/quorum-test-ledger.ts index 45b366d0b72..72eefed85dc 100644 --- a/packages/cactus-test-tooling/src/main/typescript/quorum/quorum-test-ledger.ts +++ b/packages/cactus-test-tooling/src/main/typescript/quorum/quorum-test-ledger.ts @@ -1,4 +1,3 @@ -import { Stream } from "stream"; import { EventEmitter } from "events"; import axios from "axios"; import { v4 as uuidv4 } from "uuid"; @@ -415,22 +414,25 @@ export class QuorumTestLedger implements ITestLedger { private pullContainerImage(containerNameAndTag: string): Promise { return new Promise((resolve, reject) => { const docker = new Docker(); - docker.pull(containerNameAndTag, (pullError: unknown, stream: Stream) => { - if (pullError) { - reject(pullError); - } else { - docker.modem.followProgress( - stream, - (progressError: unknown, output: unknown[]) => { - if (progressError) { - reject(progressError); - } else { - resolve(output); - } - }, - ); - } - }); + docker.pull( + containerNameAndTag, + (pullError: unknown, stream: NodeJS.ReadableStream) => { + if (pullError) { + reject(pullError); + } else { + docker.modem.followProgress( + stream, + (progressError: unknown, output: unknown[]) => { + if (progressError) { + reject(progressError); + } else { + resolve(output); + } + }, + ); + } + }, + ); }); } diff --git a/yarn.lock b/yarn.lock index 317a38e6128..71cd6b3dc11 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9263,6 +9263,7 @@ __metadata: "@types/fs-extra": "npm:9.0.13" "@types/js-yaml": "npm:4.0.3" "@types/lodash": "npm:4.14.172" + "@types/node": "npm:20.11.24" "@types/node-forge": "npm:1.3.0" "@types/ssh2": "npm:0.5.47" "@types/ssh2-streams": "npm:0.1.9" @@ -15411,6 +15412,15 @@ __metadata: languageName: node linkType: hard +"@types/node@npm:20.11.24": + version: 20.11.24 + resolution: "@types/node@npm:20.11.24" + dependencies: + undici-types: "npm:~5.26.4" + checksum: 10/7f34bfae5f9b98b9910230af4b4c52dc7fb2d1e96fdebfbc3d7576f8ab3d100076f193f9469add9e7418b455294155e7e6a028498cc5e98f9d49349875a459cf + languageName: node + linkType: hard + "@types/node@npm:20.4.7": version: 20.4.7 resolution: "@types/node@npm:20.4.7"