Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Add a test doing checks with npm for verifying the presence of package. #950

Closed
1 task
ashishdhingra opened this issue Oct 23, 2024 · 7 comments · Fixed by #964
Closed
1 task
Labels
enhancement New feature or request

Comments

@ashishdhingra
Copy link

Self-service

  • I'd be willing to implement this feature

Problem

Refer #949.

File

import { MemoryDB } from "@aws-sdk/client-memory-db";
generated by running generate:tests has incorrect import { MemoryDB } from "@aws-sdk/client-memory-db". Looks like this is not tested.

Solution

Add a test doing checks with npm for verifying the presence of package.

Alternatives

N/A

Additional context

No response

@ashishdhingra ashishdhingra added the enhancement New feature or request label Oct 23, 2024
@trivikr
Copy link
Member

trivikr commented Oct 23, 2024

Looks like this is not tested.

That's incorrect. It's tested, but the test case itself was incorrect.

@trivikr
Copy link
Member

trivikr commented Oct 23, 2024

Add a test doing checks with npm for verifying the presence of package

This would be expensive to do for each CI (PR or mainline) as it involves making a call to npm which can be intermittent. It's best to add it prior to release.

@trivikr
Copy link
Member

trivikr commented Nov 5, 2024

Since JS SDK v2 entered maintenance mode on September 8, 2024, no new clients are going to be added.

I ran a one-time check to confirm all packages from CLIENT_PACKAGE_NAMES_MAP exist

// src/test.ts
import { CLIENT_PACKAGE_NAMES_MAP } from "./transforms/v2-to-v3/config";
import { exec } from "node:child_process";
import { promisify } from "node:util";

const execAsync = promisify(exec);

(async () => {
  const failedPackages = [];
  console.log(`Checking for ${Object.keys(CLIENT_PACKAGE_NAMES_MAP).length} packages...`);
  for (const packageName of Object.values(CLIENT_PACKAGE_NAMES_MAP)) {
    const npmPackageName = `@aws-sdk/${packageName}`;
    try {
      await execAsync(`npm show ${npmPackageName} version`);
      console.log(`${npmPackageName} exists`);
    } catch (err) {
      failedPackages.push(npmPackageName);
      console.log(`${npmPackageName} does NOT exist`);
    }
  }
  console.log(`Missing packages are: [${failedPackages.join(", ")}]`);
})();
$ npx tsx src/test.ts

Checking for 384 packages...
@aws-sdk/client-acm exists
@aws-sdk/client-acm-pca exists
...
@aws-sdk/client-sage-maker-a2iruntime does NOT exist
...
@aws-sdk/client-eksauth does NOT exist
...
@aws-sdk/client-backupstorage exists
@aws-sdk/client-iot-roborunner exists
Missing packages are: [@aws-sdk/client-sage-maker-a2iruntime, @aws-sdk/client-eksauth]

@trivikr
Copy link
Member

trivikr commented Nov 5, 2024

The package names which need to be fixed:

  • @aws-sdk/client-sage-maker-a2iruntime -> @aws-sdk/client-sagemaker-a2i-runtime
  • @aws-sdk/client-eksauth -> @aws-sdk/client-eks-auth

@trivikr
Copy link
Member

trivikr commented Nov 5, 2024

Although JS SDK v2 is in maintenance mode, it's possible that CLIENT_PACKAGE_NAMES_MAP may get mistakenly updated while working on codemod.

I added a prepublishOnly script to catch validate these changes #964

@trivikr
Copy link
Member

trivikr commented Nov 7, 2024

I also wrote a test code to check if any client names are incorrect

import { CLIENT_NAMES_MAP, CLIENT_PACKAGE_NAMES_MAP } from "./transforms/v2-to-v3/config";
import { exec } from "node:child_process";
import { tmpdir } from "node:os";
import { mkdtemp } from "node:fs/promises";
import { promisify } from "node:util";

const execAsync = promisify(exec);

(async () => {
  const failedPackages = [];
  const failedClients = [];
  const tmpDir = await mkdtemp(tmpdir());

  console.log(`Checking for ${Object.keys(CLIENT_PACKAGE_NAMES_MAP).length} packages...`);
  for (const [packageKey, packageName] of Object.entries(CLIENT_PACKAGE_NAMES_MAP)) {
    const clientName = CLIENT_NAMES_MAP[packageKey];
    const npmPackageName = `@aws-sdk/${packageName}`;

    try {
      await execAsync(`npm show ${npmPackageName} version`);
      console.log(`${npmPackageName} exists`);

      try {
        const execOptions = { cwd: tmpDir };
        await execAsync("rm -rf *", execOptions);
        await execAsync(`npm install ${npmPackageName}`, execOptions);
        await execAsync(
          `echo 'import { ${clientName} } from "${npmPackageName}"' > index.mjs`,
          execOptions
        );
        await execAsync("node index.mjs", execOptions);
      } catch (err) {
        failedClients.push(clientName);
        console.log(`${clientName} failed to import`);
      }
    } catch (err) {
      failedPackages.push(npmPackageName);
      console.log(`${npmPackageName} does NOT exist`);
    }
  }
  console.log(`Missing packages are: [${failedPackages.join(", ")}]`);
  console.log(`Missing clients are: [${failedClients.join(", ")}]`);
})();

It detected two package names whose clients were removed as they're no longer supported #968

Copy link
Contributor

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants