test(test tooling): add DamlTestLedger implementation #3462

raynatopedrajeta · 2024-08-07T03:46:12Z

Commit to be reviewed

test(test tooling): add DamlTestLedger implementation

Primary Changes
---------------
1. Create a test tooling class for DAML AIO Image

Fixes #3435

Pull Request Requirements

Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

jagpreetsinghsasan · 2024-08-07T05:44:16Z

@raynatopedrajeta please remove the daml connector folder (containing the package.json) and fix the failing CI issues

petermetz

@raynatopedrajeta Plus one to what @jagpreetsinghsasan said plus the usual nits like squashing the commits and having a PR title + commit subject that isn't going to end up in the production changelog.md file (feat will make it so that it will so it needs to be test even if it is technically a feature of the testing capabilities)

jagpreetsinghsasan · 2024-08-12T03:05:14Z

@raynatopedrajeta Plus one to what @jagpreetsinghsasan said plus the usual nits like squashing the commits and having a PR title + commit subject that isn't going to end up in the production changelog.md file (feat will make it so that it will so it needs to be test even if it is technically a feature of the testing capabilities)

True @petermetz , although Rayn has 2 commits in this PR as one of the commits if of the previous PR (This PR is in continuation to #3411 , thus 2 commits)

RafaelAPB

Generally LGTM. I've left a few comments. Thanks for your contribution!

RafaelAPB · 2024-08-13T13:57:05Z

packages/cactus-test-tooling/src/main/typescript/public-api.ts

@@ -13,6 +13,17 @@ export {
  IBesuMpTestLedgerOptions,
 } from "./besu/besu-mp-test-ledger";

+export {
+  DamlTestLedger
+  // IDamlTestLedgerConstructorOptions,


Why are not the option objects, etc, being exported?

RafaelAPB · 2024-08-13T13:58:35Z

packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts

+
+  public async getDamlApiHost(): Promise<string> {
+    const ipAddress = "127.0.0.1";
+    // const port = await Containers.getPublicPort(rpcApiWsPort, containerInfo);


If the exposed port for the used DAML image is 7575, we may delete this comment

Hello @RafaelAPB ,
Deleted this part. Thanks!

Rayn

RafaelAPB · 2024-08-13T14:00:16Z

packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts

+    });
+  }
+
+  public async waitForHealthCheck(timeoutMs = 360000): Promise<void> {


5 minutes looks like a very big timeout; perhaps reduce it accordin g to other test ledgers?

Hello @RafaelAPB ,
At the moment I set this to 2 minutes. However I will still do some runs if this can still be reduced. Thanks!

Cheers!
Rayn

RafaelAPB · 2024-08-13T14:00:56Z

packages/cactus-test-tooling/src/main/typescript/public-api.ts

+} from "./daml/daml-test-ledger";
+
+// export {
+//   BesuMpTestLedger,


is this a typo/mistake? Why are we commenting the exporting of the besu test ledger?

Hello @RafaelAPB ,
Yes this is just a typo mistake. I wasn't able to switch this to draft PR before I pushed this :)

Cheers!
Rayn

RafaelAPB · 2024-08-13T14:06:49Z

packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts

@@ -0,0 +1,328 @@
+import Docker, { Container, ContainerInfo } from "dockerode";


Please add tests in https://github.com/hyperledger/cacti/tree/main/packages/cactus-test-tooling/src/test/typescript/integration/damn-test-ledger where 1) we can instatiate the test ledger successfully; 2) we create multiple test ledgers with different ports. This is useful to check that the test ledger is flexible enough to be exposed in different ports. You have an example here packages/cactus-test-tooling/src/test/typescript/integration/substrate/substrate-test-ledger-constructor.test.ts

I belive it is a good practice to use const pruning = pruneDockerAllIfGithubAction({ logLevel }); to prune hanging containers in the CI, but please double check with @petermetz.

@RafaelAPB Plus one to everything you are saying. Thank you so much for the reviews, its extremely helpful to have more watchful eyes on the code!!

RafaelAPB · 2024-08-13T14:07:15Z

...us-plugin-ledger-connector-daml/src/test/typescript/integration/daml-get-transaction.test.ts

+
+  describe("daml test transaction", () => {
+    it("daml test transaction", async () => {
+      console.log("Initial run success")


Please populate the proposed test

Hello @RafaelAPB ,
As per my conversation with @jagpreetsinghsasan , there will be a separate ticket for this:
#3489

Cheers!
Rayn

Fair enough :)

@raynatopedrajeta Could you please answer to my other queries?

Hello @RafaelAPB ,
Replied on your queries.

Thank you!
Rayn

raynatopedrajeta · 2024-09-05T07:01:28Z

@raynatopedrajeta please fix the failing CI tests

Hello @jagpreetsinghsasan ,

Review point has been addressed.

Cheers!
Rayn

petermetz

@raynatopedrajeta I'll publish a build of the image from the PR on the official repo shortly. Could you please then update the source code to use that image instead?
Also, could you remove the entry "cactuts" from the spell checker configuration? I'm assuming it was a typo while typing "cacti" so we shouldn't add the typo itself in the list of correct words.

petermetz · 2024-09-08T07:59:51Z

Suggested edit:

diff --git a/packages/cactus-plugin-ledger-connector-daml/src/test/typescript/integration/daml-get-transaction.test.ts b/packages/cactus-plugin-ledger-connector-daml/src/test/typescript/integration/daml-get-transaction.test.ts
index 490c5ff23..b4d3fb42d 100644
--- a/packages/cactus-plugin-ledger-connector-daml/src/test/typescript/integration/daml-get-transaction.test.ts
+++ b/packages/cactus-plugin-ledger-connector-daml/src/test/typescript/integration/daml-get-transaction.test.ts
@@ -6,11 +6,10 @@ import {
   LoggerProvider,
   Logger,
 } from "@hyperledger/cactus-common";
+
+import { DAML_TEST_LEDGER_DEFAULT_OPTIONS } from "@hyperledger/cactus-test-tooling";
 import { pruneDockerAllIfGithubAction } from "../../../../../../packages/cactus-test-tooling/src/main/typescript/github-actions/prune-docker-all-if-github-action";
 //Constants
-const containerImageVersion = "latest";
-const containerImageName = "cactuts/daml-all-in-one";
-const rpcApiHttpPort = 7575;
 
 const testLogLevel: LogLevelDesc = "info";
 
@@ -28,12 +27,12 @@ describe("PluginLedgerConnectorDAML", () => {
     await pruneDockerAllIfGithubAction({ logLevel: testLogLevel });
 
     damlTestLedger = new DamlTestLedger({
-      containerImageVersion,
-      containerImageName,
-      rpcApiHttpPort,
+      imageName: DAML_TEST_LEDGER_DEFAULT_OPTIONS.imageName,
+      imageVersion: DAML_TEST_LEDGER_DEFAULT_OPTIONS.imageVersion,
+      rpcApiHttpPort: DAML_TEST_LEDGER_DEFAULT_OPTIONS.rpcApiHttpPort,
     });
 
-    log.debug("DAML image:", damlTestLedger.containerImageName);
+    log.debug("DAML image:", damlTestLedger.imageName);
     expect(damlTestLedger).toBeTruthy();
     await damlTestLedger.start();
   });
diff --git a/packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts b/packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts
index f1d6ea253..fbe666c8b 100644
--- a/packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts
+++ b/packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts
@@ -12,24 +12,26 @@ import { ITestLedger } from "../i-test-ledger";
 import { Streams } from "../common/streams";
 import { Containers } from "../common/containers";
 
-export interface DAMLTestLedgerConstructorOptions {
-  containerImageVersion?: string;
-  containerImageName?: string;
+export interface IDamlTestLedgerOptions {
+  imageVersion?: string;
+  imageName?: string;
   rpcApiHttpPort?: number;
   logLevel?: LogLevelDesc;
   emitContainerLogs?: boolean;
 }
 
-export const DAML_TEST_LEDGER_DEFAULT_OPTIONS = Object.freeze({
-  containerImageVersion: "latest",
-  containerImageName: "cactuts/daml-all-in-one",
+const DEFAULTS = Object.freeze({
+  imageVersion: "2024-09-08T07-40-07-dev-2cc217b7a",
+  imageName: "ghcr.io/hyperledger/cacti-daml-all-in-one",
   rpcApiHttpPort: 7575,
 });
 
+export const DAML_TEST_LEDGER_DEFAULT_OPTIONS = DEFAULTS;
+
 export const DAML_TEST_LEDGER_OPTIONS_JOI_SCHEMA: Joi.Schema =
   Joi.object().keys({
-    containerImageVersion: Joi.string().min(5).required(),
-    containerImageName: Joi.string().min(1).required(),
+    imageVersion: Joi.string().min(5).required(),
+    imageName: Joi.string().min(1).required(),
     rpcApiHttpPort: Joi.number()
       .integer()
       .positive()
@@ -39,8 +41,8 @@ export const DAML_TEST_LEDGER_OPTIONS_JOI_SCHEMA: Joi.Schema =
   });
 
 export class DamlTestLedger implements ITestLedger {
-  public readonly containerImageVersion: string;
-  public readonly containerImageName: string;
+  public readonly imageVersion: string;
+  public readonly imageName: string;
   public readonly rpcApiHttpPort: number;
   public readonly emitContainerLogs: boolean;
 
@@ -48,26 +50,21 @@ export class DamlTestLedger implements ITestLedger {
   private container: Container | undefined;
   private containerId: string | undefined;
 
-  constructor(public readonly options: DAMLTestLedgerConstructorOptions = {}) {
-    if (!options) {
+  constructor(public readonly opts?: IDamlTestLedgerOptions) {
+    if (!opts) {
       throw new TypeError(`DAMLTestLedger#ctor options was falsy.`);
     }
-    this.containerImageVersion =
-      options.containerImageVersion ||
-      DAML_TEST_LEDGER_DEFAULT_OPTIONS.containerImageVersion;
-    this.containerImageName =
-      options.containerImageName ||
-      DAML_TEST_LEDGER_DEFAULT_OPTIONS.containerImageName;
-    this.rpcApiHttpPort =
-      options.rpcApiHttpPort || DAML_TEST_LEDGER_DEFAULT_OPTIONS.rpcApiHttpPort;
-
-    this.emitContainerLogs = Bools.isBooleanStrict(options.emitContainerLogs)
-      ? (options.emitContainerLogs as boolean)
+    this.imageVersion = opts.imageVersion || DEFAULTS.imageVersion;
+    this.imageName = opts.imageName || DEFAULTS.imageName;
+    this.rpcApiHttpPort = opts.rpcApiHttpPort || DEFAULTS.rpcApiHttpPort;
+
+    this.emitContainerLogs = Bools.isBooleanStrict(opts.emitContainerLogs)
+      ? (opts.emitContainerLogs as boolean)
       : true;
 
     this.validateConstructorOptions();
     const label = "daml-test-ledger";
-    const level = options.logLevel || "INFO";
+    const level = opts.logLevel || "INFO";
     this.log = LoggerProvider.getOrCreate({ level, label });
   }
 
@@ -81,7 +78,7 @@ export class DamlTestLedger implements ITestLedger {
   }
 
   public getContainerImageName(): string {
-    return `${this.containerImageName}:${this.containerImageVersion}`;
+    return `${this.imageName}:${this.imageVersion}`;
   }
 
   public async getRpcApiHttpHost(): Promise<string> {
@@ -312,7 +309,7 @@ export class DamlTestLedger implements ITestLedger {
         return NetworkSettings.Networks[networkNames[0]].IPAddress;
       }
     } else {
-      throw new Error(`${fnTag} cannot find image: ${this.containerImageName}`);
+      throw new Error(`${fnTag} cannot find image: ${this.imageName}`);
     }
   }
 
@@ -340,8 +337,8 @@ export class DamlTestLedger implements ITestLedger {
 
   private validateConstructorOptions(): void {
     const validationResult = DAML_TEST_LEDGER_OPTIONS_JOI_SCHEMA.validate({
-      containerImageVersion: this.containerImageVersion,
-      containerImageName: this.containerImageName,
+      imageVersion: this.imageVersion,
+      imageName: this.imageName,
       rpcApiHttpPort: this.rpcApiHttpPort,
     });
 
diff --git a/packages/cactus-test-tooling/src/main/typescript/public-api.ts b/packages/cactus-test-tooling/src/main/typescript/public-api.ts
index d39057572..daba410ae 100755
--- a/packages/cactus-test-tooling/src/main/typescript/public-api.ts
+++ b/packages/cactus-test-tooling/src/main/typescript/public-api.ts
@@ -13,7 +13,11 @@ export {
   IBesuMpTestLedgerOptions,
 } from "./besu/besu-mp-test-ledger";
 
-export { DamlTestLedger } from "./daml/daml-test-ledger";
+export {
+  DamlTestLedger,
+  DAML_TEST_LEDGER_DEFAULT_OPTIONS,
+  IDamlTestLedgerOptions,
+} from "./daml/daml-test-ledger";
 
 export {
   CordaTestLedger,
diff --git a/tools/docker/daml-all-in-one/Dockerfile b/tools/docker/daml-all-in-one/Dockerfile
index d83f3cf80..80195b18a 100644
--- a/tools/docker/daml-all-in-one/Dockerfile
+++ b/tools/docker/daml-all-in-one/Dockerfile
@@ -1,15 +1,17 @@
 FROM ubuntu:22.04
 
-RUN apt update
-RUN apt install curl openjdk-21-jdk -y
+LABEL org.opencontainers.image.source = "https://github.com/hyperledger/cacti"
+
+RUN apt update && \
+    apt install --no-install-recommends curl openjdk-21-jdk supervisor openssl xxd -y
 
 # Download and install DAML SDK 2.9.3
 RUN curl -L https://github.com/digital-asset/daml/releases/download/v2.9.3/daml-sdk-2.9.3-linux.tar.gz | tar -xz -C /opt && \
     cd /opt/sdk-2.9.3 && \
-    ./install.sh
+    ./install.sh && \
+    rm -rf /opt/sdk-2.9.3/
 
 ENV PATH="/root/.daml/bin:${PATH}"
-RUN apt-get install xxd
 RUN daml new quickstart --template quickstart-java
 WORKDIR /quickstart
 
@@ -17,12 +19,10 @@ WORKDIR /quickstart
 RUN echo '{"server": {"address": "0.0.0.0","port": 7575},"ledger-api": {"address": "0.0.0.0","port": 6865}}' > json-api-app.conf
 
 # Run the auto generation of Authorization Bearer Token
-RUN apt-get update && apt-get install -y openssl
 COPY generate-jwt-token.sh /quickstart/generate-jwt-token.sh
 RUN chmod +x /quickstart/generate-jwt-token.sh
 RUN /quickstart/generate-jwt-token.sh
 
-RUN apt-get update && apt-get install -y supervisor
 RUN mkdir -p /var/log/supervisor
 COPY supervisord.conf /etc/supervisord.conf

petermetz · 2024-09-08T08:00:31Z

@raynatopedrajeta See my suggestions above and also this: https://github.com/hyperledger/cacti/pkgs/container/cacti-daml-all-in-one

raynatopedrajeta · 2024-09-10T05:47:25Z

@raynatopedrajeta I'll publish a build of the image from the PR on the official repo shortly. Could you please then update the source code to use that image instead? Also, could you remove the entry "cactuts" from the spell checker configuration? I'm assuming it was a typo while typing "cacti" so we shouldn't add the typo itself in the list of correct words.

Hello @petermetz,
This is noted Peter, we temporarily included "cactuts" on cspell in order for the program to proceed but we are planning to remove this once the image has been created :)

Cheers!
Rayn

petermetz · 2024-09-10T06:16:52Z

@raynatopedrajeta Got it thank you! I did create and upload the image (link in my previous comment). The suggested edits also contain the source code change that updates the code to use the image I uploaded so if you accept the suggested edits as is then it should be taken care of

RafaelAPB

LGTM, thanks for your contribution

petermetz

@raynatopedrajeta We are getting close, thank you for the changes so far!
Please make sure to

Address @RafaelAPB 's earlier request regarding the test cases. Right now if I read it right there is zero coverage for the DamlTestLedger class.
Please update the PR title and the commit message to reflect the nature of the change. It is a feature but not added to the production code and therefore it should have a type of test. Something like: test(test-tooling): add DamlTestLedger implementation is what I would recommend. The reason why this is important is because we are trying to improve the quality of the release notes (the CHANGELOG.md files) which are automatically generated from the commit messages based on their subject line.
Please see: test(test tooling): add DamlTestLedger implementation #3462 (comment)

petermetz · 2024-09-08T06:32:22Z

packages/cactus-test-tooling/src/main/typescript/daml/daml-test-ledger.ts

+    return `http://${ipAddress}:${hostPort}`;
+  }
+
+  public async getDamlApiHost(): Promise<string> {


@raynatopedrajeta getDamlApiHost() seems like a broken version getRpcApiHttpHost() are they both necessary or can getDamlApiHost() be removed?

Hello @petermetz ,
Here are my responses per item:

As per @jagpreetsinghsasan , test scenario for DamlTestledger(daml-get-transaction.ts) will move on the next item, feat(daml): creation of connector class #3489
2 and 3. Changes have been applied.

Cheers!
Rayn

Primary Changes --------------- 1. Create a test tooling class for DAML AIO Image Fixes hyperledger#3435 Signed-off-by: raynato.c.pedrajeta <raynato.c.pedrajeta@accenture.com>

raynatopedrajeta requested review from petermetz, takeutak, izuru0, jagpreetsinghsasan, VRamakrishna, sandeepnRES and outSH as code owners August 7, 2024 03:46

petermetz requested changes Aug 7, 2024

View reviewed changes

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 4 times, most recently from 2c01942 to 11847b2 Compare August 13, 2024 13:26

RafaelAPB requested changes Aug 13, 2024

View reviewed changes

raynatopedrajeta marked this pull request as draft August 15, 2024 06:40

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 6 times, most recently from cca6931 to bf06a08 Compare August 20, 2024 06:44

raynatopedrajeta marked this pull request as ready for review August 20, 2024 06:56

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 6 times, most recently from ef4e15d to 8c317eb Compare August 20, 2024 07:36

raynatopedrajeta requested a review from RafaelAPB August 20, 2024 07:56

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 3 times, most recently from 7bc413e to 2cc217b Compare September 5, 2024 06:50

raynatopedrajeta requested a review from petermetz September 5, 2024 07:01

petermetz requested changes Sep 8, 2024

View reviewed changes

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 3 times, most recently from 836be12 to 28d84cd Compare September 16, 2024 06:57

RafaelAPB approved these changes Sep 16, 2024

View reviewed changes

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 2 times, most recently from 0db8b9e to b83dc66 Compare September 18, 2024 07:08

raynatopedrajeta requested a review from petermetz September 19, 2024 03:38

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch from b83dc66 to a3fe238 Compare September 19, 2024 06:58

petermetz requested changes Sep 19, 2024

View reviewed changes

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch 5 times, most recently from f90a485 to 6d5bc90 Compare September 20, 2024 09:49

raynatopedrajeta changed the title ~~feat(daml): test tooling class~~ test(daml): test tooling Sep 20, 2024

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch from 6d5bc90 to 57992d5 Compare September 20, 2024 10:00

raynatopedrajeta changed the title ~~test(daml): test tooling~~ test(test tooling): add DamlTestLedger implementation Sep 20, 2024

test(test tooling): add DamlTestLedger implementation

0f9db67

Primary Changes --------------- 1. Create a test tooling class for DAML AIO Image Fixes hyperledger#3435 Signed-off-by: raynato.c.pedrajeta <raynato.c.pedrajeta@accenture.com>

raynatopedrajeta force-pushed the raynatopedrajeta/issue3435 branch from 57992d5 to 0f9db67 Compare September 20, 2024 10:21

raynatopedrajeta requested a review from petermetz September 20, 2024 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(test tooling): add DamlTestLedger implementation #3462

test(test tooling): add DamlTestLedger implementation #3462

raynatopedrajeta commented Aug 7, 2024 •

edited

Loading

jagpreetsinghsasan commented Aug 7, 2024

petermetz left a comment

jagpreetsinghsasan commented Aug 12, 2024

RafaelAPB left a comment

RafaelAPB Aug 13, 2024

RafaelAPB Aug 13, 2024

raynatopedrajeta Aug 26, 2024

RafaelAPB Aug 13, 2024

raynatopedrajeta Aug 26, 2024

RafaelAPB Aug 13, 2024

raynatopedrajeta Aug 26, 2024

RafaelAPB Aug 13, 2024

petermetz Aug 13, 2024

RafaelAPB Aug 13, 2024

raynatopedrajeta Aug 20, 2024

RafaelAPB Aug 23, 2024

RafaelAPB Aug 23, 2024

raynatopedrajeta Aug 26, 2024

raynatopedrajeta commented Sep 5, 2024

petermetz left a comment

petermetz commented Sep 8, 2024

petermetz commented Sep 8, 2024

raynatopedrajeta commented Sep 10, 2024

petermetz commented Sep 10, 2024

RafaelAPB left a comment

petermetz left a comment •

edited

Loading

petermetz Sep 8, 2024

raynatopedrajeta Sep 20, 2024

		@@ -0,0 +1,328 @@
		import Docker, { Container, ContainerInfo } from "dockerode";

test(test tooling): add DamlTestLedger implementation #3462

Are you sure you want to change the base?

test(test tooling): add DamlTestLedger implementation #3462

Conversation

raynatopedrajeta commented Aug 7, 2024 • edited Loading

Commit to be reviewed

jagpreetsinghsasan commented Aug 7, 2024

petermetz left a comment

Choose a reason for hiding this comment

jagpreetsinghsasan commented Aug 12, 2024

RafaelAPB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raynatopedrajeta commented Sep 5, 2024

petermetz left a comment

Choose a reason for hiding this comment

petermetz commented Sep 8, 2024

petermetz commented Sep 8, 2024

raynatopedrajeta commented Sep 10, 2024

petermetz commented Sep 10, 2024

RafaelAPB left a comment

Choose a reason for hiding this comment

petermetz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raynatopedrajeta commented Aug 7, 2024 •

edited

Loading

petermetz left a comment •

edited

Loading