code-pushup / cli

A CLI to run all kinds of code quality measurements to align your team with company goals
https://code-pushup.dev
MIT License
252 stars 14 forks source link

test: consolidate e2e tests setup #643

Open getlarge opened 6 months ago

getlarge commented 6 months ago

@BioPhoton @matejchalk It looks like there is a problem with the e2e tests setup:

Which leads to race conditions and conflicts with package publishing/installation.

Originally posted by @getlarge in https://github.com/code-pushup/cli/issues/639#issuecomment-2072213132

getlarge commented 6 months ago

We started to dig on this problem with @BioPhoton and found out that:

There are still other factors to consider that might be related to environment variables and/or NPM configuration conflicts, since the e2e tests still fail when run in parallel when using a random Verdaccio storage location.

I looked for how Nx handles the local registry for their end-to-end tests and they use similar logic in a global setup hook to create unique storage path for each e2e project by joining process.env.NX_TASK_TARGET_PROJECT to the path.

Their setup differs in the way they run the e2e tests in the CI, instead of running tests in parallel in the same parent process, they run each test in a different runner using a matrix. This way they isolate environment variables, NPM and Git configuration concurrent access.

getlarge commented 6 months ago

On the same topic, the generic global-setup.e2e.ts and the cli-e2e test suite are currently coupled. The functions setupTestFolder and teardownTestFolder are called with the argument tmp/e2e, which seems specific to the cli-e2e moreover, if multiple projects would use the same global-setup.e2e.ts the folder could be deleted while it is still being used by another test suite which is pending. Consider moving these functions in beforeAll and afterAll hooks?

Similar problem with teardownTestFolder('tmp/local-registry') which would delete the local registry storage while one might still be running for another suite (even though it seems complex to have multiple registry in a single process). Also, the folder could be cleared when calling the local-registry target:

"local-registry": {
      "executor": "@nx/js:verdaccio",
      "options": {
        "port": 4873,
        "config": ".verdaccio/config.yml",
        // added clear option
        "clear": true
      }
    },
getlarge commented 6 months ago

Leaving this for posterity, if anyone tries to tackle the concurrent npm publishing, we created a workaround to avoid throwing an exception when a package is already published in tools/scripts/publish.mjs.


// ...
/**
 * @description this logic is to avoid publishing the same version of the package and fail silently in case the package is already installed*
 */
function checkIfPackageIsPublished() {
  // or use     execSync(`npm view ${name}@${version}`);
  try {
    const output = execSync(`npm ls ${json.name} --json`, {
      encoding: 'utf-8',
    });
    const { dependencies } = JSON.parse(output);
    console.log({
      version,
      dependency: dependencies[json.name],
    });
    return !!(
      dependencies &&
      dependencies[json.name] &&
      dependencies[json.name].version === version
    );
  } catch (e) {
    return false;
  }
}

// Updating the version in "package.json" before publishing
try {
  const json = JSON.parse(readFileSync(`package.json`).toString());
  json.version = version;
  const isPackagePublished = checkIfPackageIsPublished();
  if (isPackagePublished) {
    console.warn(
      `Package "${json.name}" is already installed in the workspace`,
    );
    process.exit(0);
  }
  writeFileSync(`package.json`, JSON.stringify(json, null, 2));
} catch (e) {
  console.error(e);
  console.error(`Error reading package.json file from library build output.`);
}

// Execute "npm publish" to publish
execSync(`npm publish --access public --tag ${tag}`);