ci(github): add check to validate exported types being correct

[ruzell22]

Commit to be reviewed

---

ci(github): add check to validate exported types being correct

Primary Changes
---------------
1. Added get-all-tgz-path.ts to get all tgz files path
2. Added run-attw-on-tgz.ts to run attw on each tgz filepath
3. Added are-the-types-wrong as part of the custom-checks mechanism

Fixes: #3140

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.

[outSH]

Thanks for the PR! I've left few comments, plese re-request the review once they're resolved :)

[petermetz]

@ruzell22 Please add the check to b e a part of the custom-checks mechanism and then we can delete the standalone github action workflow (which saves on resources)

[petermetz]

@ruzell22 This is looking closer now, but what I need is that if we run yarn custom-checks then the attw check is executed as part of it. Right now it's a standalone script only (it's good that it can be executed by itself as well but it also needs to be a part of the custom-checks so that in ci.yaml when the custom-checks are executed this also runs)

[outSH]

LGTM, thank you

[petermetz]

@ruzell22 We are getting closer! Now I tested it and found 2 more issues:

1. The error messages you return are just the paths of the .tgz files with issues but there's nothin in there about what the problem is. Instead of logging the execCommand output to stdout, return it as a string that can be appended to the custom-check errors so that the error messages at the end contain the information we need.
2. Please also abide by the existing conventions of the error messages like prefixing them with ERROR if I remember correctly.
3. Right now the glob captures all .tgz files meaning that the check is failing on my machine because I have some leftover .tgz files from an earlier release (2.0.0-rc.3). Please add an extra step that first wipes all the matching .tgz files BEFORE it runs attw and the packaging step (so that we always start with a clean slate with the only .tgz file being in there the one that we need)

[petermetz]

@ruzell22 One more thing: Please apply the automatic formatting to the .ts files that you've added. There are linter errors in there about let vs. const variable declarations for example.

[petermetz]

@ruzell22 Are you still working on this?

[ruzell22]

@petermetz yes, I am. I currently have the codes ready to be push with the added extra step to wipe all the existing .tgz files first and also the additional information on the .tgz files with error after running attw. However, I just noticed something when testing before pushing the code wherein on the previous test without the wiping, it patches and detects 71 tgz files. After I put the wipe before patching and detecting tgz files, it can only detect 46 file paths.

[petermetz]

@petermetz yes, I am. I currently have the codes ready to be push with the added extra step to wipe all the existing .tgz files first and also the additional information on the .tgz files with error after running attw. However, I just noticed something when testing before pushing the code wherein on the previous test without the wiping, it patches and detects 71 tgz files. After I put the wipe before patching and detecting tgz files, it can only detect 46 file paths.

@ruzell22 Got it, thank you for the update! Take your time, I just wanted to make sure that the PR is not being held up by something.

[petermetz]

@ruzell22 Now it's crashing on my machine and the logging is unfortunately hiding the full output that you get from the child processes (attached log file 1).

I recommend refactoring the code in a way that streams the logs of the child processes directly to the stdout/stderr of the main process instead (attached code modified)

If you run it like that (with streamed output logs), then it at least gives you the full output which shows some logs that are just "undefined" which indicates that you still have some errors that aren't handled correctly.

The other thing I've seen is that there quite a few packages that are failing the ATTW checks. We don't have to fix these as part of this pull request, but we will need to fix them before this PR gets merged because otherwise the CI would break down because these new checks would make the mandatory checks fail for everyone in all PRs.

So, there might be still multiple issues but let's just take it one at a time, you wanna make sure that the error messages include the details (e.g. on the screenshot below you want to capture the information in red in the section of the logs where I marked it yellow).
This will ensure that the error details don't get lost when people have failures.

2024-11-11-cacti-attw-custom-checks-streamed-output.crash.log
2024-11-11-cacti-custom-check-attw.crash.log

tools/custom-checks/run-attw-on-tgz.ts

import esMain from "es-main";
import { getAllTgzPath } from "./get-all-tgz-path";
import { exit } from "process";
import { rm } from "fs";
import { spawn } from "child_process";

// New function to stream subprocess output in real-time
function spawnPromise(
  command: string,
  args: string[],
  options = {},
): Promise<void> {
  return new Promise((resolve, reject) => {
    const child = spawn(command, args, { shell: true, ...options });

    // Stream output to main process stdout and stderr
    child.stdout.pipe(process.stdout);
    child.stderr.pipe(process.stderr);

    child.on("close", (code) => {
      if (code === 0) {
        resolve();
      } else {
        reject(new Error(`Process exited with code ${code}`));
      }
    });

    child.on("error", (err) => reject(err));
  });
}

async function cleanUpTgzFiles(): Promise<void> {
  const TAG = "[tools/custom-checks/run-attw-on-tgz.ts]";
  console.log(`${TAG} Cleaning up existing .tgz files...`);

  const { relativePaths: tgzFilesRelative } = await getAllTgzPath();

  for (const filePath of tgzFilesRelative) {
    await rm(filePath, { recursive: true, force: true }, () => {});
    console.log(`${TAG} Deleted ${filePath}`);
  }
}

export async function runAttwOnTgz(): Promise<[boolean, string[]]> {
  await cleanUpTgzFiles();

  const TAG = "[tools/custom-checks/run-attw-on-tgz.ts]";
  await execCommand("yarn lerna exec 'npm pack'", true);
  console.log(`${TAG} Packaging .tgz files`);

  console.log(`${TAG} Fetching .tgz file paths.`);
  const { relativePaths: tgzFilesRelative } = await getAllTgzPath();

  const attwFailedPackages: string[] = [];

  for (const filePath of tgzFilesRelative) {
    try {
      const output = await execCommand("attw", filePath);
      console.log(output);
    } catch (error: any) {
      attwFailedPackages.push(
        `ERROR ${filePath}: ${error.message} ${error.output}`,
      );
    }
  }

  const success = attwFailedPackages.length === 0;
  return [success, attwFailedPackages];
}

async function execCommand(
  binaryName: string,
  argument: string | boolean,
): Promise<void> {
  let command;
  if (typeof argument === "boolean") {
    command = binaryName;
  } else if (typeof argument === "string") {
    command = `${binaryName} ./${argument}`;
  } else {
    throw new Error("Invalid arguments for execCommand");
  }

  await spawnPromise(command, []);
}

if (esMain(import.meta)) {
  const [success, attwFailedPackages] = await runAttwOnTgz();
  if (!success) {
    console.log("Types are wrong for these packages:");
    console.log(attwFailedPackages);
    exit(1);
  }
  exit(0);
}

[ruzell22]

@petermetz on the latest push, there are no undefined output now. Also, I have created a new issue #3623 that relates to issue #3140 for fixing the packages that are failing with ATTW check in custom-checks.

[petermetz]

@ruzell22 Yes! We are looking good! Thank you!
Now for the final part, since we don't want to hold this PR up by waiting for the broken type exports to be fixed, what I'll ask just to resolve the circular dependency between the broken type exports and the passing CI checks is to:

1. Exclude the broken packages from the check for now, in a way that there is a single entry in the ignore patterns for each package AND
2. There is a comment above each ignore pattern referencing a GitHub issue that was created with the specific task of fixing the broken types of that specific package.
3. Once these changes are in place, please re-request review and then we can merge the check as it should be passing at that point.

What this achieves is that we can merge this PR today without melting down the CI and yet for new packages we get the type export validation out of the box working.

We also get the benefit that from now on this check will guard against mistakes where someone breaks the otherwise currently correct type exports for packages where it's good.

I've collected the list of ignore patterns that you'll need but please create the issues for each separately and then add the comments for each line with the link to the specific issue.
Also leave a comment explaining everything I just talked about. The guiding principle there should be that whoever stumbles upon those ignore patterns should be able to figure out what is happening without having to ask you or me for historical context. Ideally if they would like to work on fixing those type exports they can get cracking with it right after having seen the comments. This is the general principle behind it that gives us a better chance longer term of having a more maintainable codebase.

The ignore paths you'll need in the globby options:

tools/custom-checks/get-all-tgz-path.ts

  const globbyOpts: GlobbyOptions = {
    cwd: PROJECT_DIR,
    onlyFiles: true,
    expandDirectories: false,
    ignore: [
      "**/node_modules",
      "weaver/core/drivers/fabric-driver/hyperledger-cacti-weaver-driver-fabric-*.tgz",
      "weaver/core/identity-management/iin-agent/hyperledger-cacti-weaver-iin-agent-*.tgz",
      "weaver/sdks/fabric/interoperation-node-sdk/hyperledger-cacti-weaver-sdk-fabric-*.tgz",
      "packages/cacti-plugin-weaver-driver-fabric/src/main/typescript/hyperledger-cacti-weaver-driver-fabric-*.tgz",
      "packages/cacti-plugin-copm-fabric/hyperledger-cacti-cacti-plugin-copm-fabric-*.tgz",
      "packages/cacti-ledger-browser/hyperledger-cacti-ledger-browser-*.tgz",
      "examples/cactus-example-cbdc-bridging-frontend/hyperledger-cacti-example-cbdc-bridging-frontend-*.tgz",
      "examples/cactus-common-example-server/hyperledger-cactus-common-example-server-*.tgz",
      "packages/cactus-verifier-client/hyperledger-cactus-verifier-client-*.tgz",
      "packages/cactus-plugin-ledger-connector-polkadot/hyperledger-cactus-plugin-ledger-connector-polkadot-*.tgz",
      "packages/cactus-common/hyperledger-cactus-common-*.tgz",
    ],
  };

[petermetz]

LGTM