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: Easier logger override for wrapping Chromatic-node inside an NX-executor #842

Closed
work933k opened this issue Oct 24, 2023 · 6 comments
Labels
needs triage Tracking: Issue needs confirmation

Comments

@work933k
Copy link
Contributor

work933k commented Oct 24, 2023

Feature request

I've created a NX-executor for running Chromatic-node. At this moment the log is written outside of the context of my NX-executor. If i would be able to wrap/capture the output of Chromatic logging. This would allow me to put all output in a single collapsible-panel in the CI-pipeline.

I know that it's possible to capture the output of the Chromatic-cli-version, but then i don't have the detail-output which i now have by using the Chromatic-node.

Proposed solution

If possible: provide an optional logger-parameter in the "run"-function instead of calling createLogger.
https://github.com/chromaui/chromatic-cli/blob/526c06583a3f0388583f320f5446c8a74838a1ff/node-src/index.ts#L79C13-L79C13

Alternative solutions

I tried to override "console.log/console.error/...", but that resulted in a stack-overflow. Probably because the logger wrote to itself and caused recursion.

I think i could also copy the "run"-function and tweak it., before calling the runAll-function. But that's quite a dirty hack.

Additional context

Below is the code of my custom executor. In the executor i check for the result of Chromatic and write a special error-string to the console for Azure DevOps. This special string will become visible in the build-run-overview, which is more user-friendly.

export default async function runExecutor(options: ChromaticExecutorSchema, context: ExecutorContext): Promise<any> {
  const outputDir = getBuildTargetOutputDir(options, context);

  try {
    const chromaticOptions: ChromaticOptions = {} as ChromaticOptions;

    if (options.onlyChanged != '') {
      chromaticOptions.onlyChanged = options.onlyChanged;
    }

    if (options.exitZeroOnChanges != '') {
      chromaticOptions.exitZeroOnChanges = options.exitZeroOnChanges;
    }

    if (options.exitOnceUploaded != '') {
      chromaticOptions.exitOnceUploaded = options.exitOnceUploaded;
    }

    if (options.autoAcceptChanges != '') {
      chromaticOptions.autoAcceptChanges = options.autoAcceptChanges;
    }

    if (options.allowConsoleErrors) {
      chromaticOptions.allowConsoleErrors = true;
    }

    if (options.token) {
      chromaticOptions.projectToken = options.token;
    }

    chromaticOptions.storybookBuildDir = outputDir;

    const result = await ChromaticRun({ options: chromaticOptions });
    // Temporary result output to tweak further.
    logger.log('Result:', result);

    if (result.code != 0) {
      if (result.url && !process.env['CI']) {
        logger.log(`##vso[task.logissue type=error]Chromatic result for "${context.projectName}": ${result.url}`);
      }
      return { success: false };
    }
  } catch (e) {
    logger.error(e);
    return { success: false };
  }
  return { success: true };
}
@work933k work933k added the needs triage Tracking: Issue needs confirmation label Oct 24, 2023
@work933k work933k changed the title Feature: Easier logger override Feature: Easier logger override for wrapping Chromatic-node inside an NX-executor Oct 24, 2023
@ghengeveld
Copy link
Member

With v7.5.0. we introduced chromatic.log, a file where logs are written to. Would that work for you? You could tail -f chromatic.log, although in interactive mode (TTY) it won't contain any of the step progress until the build is complete.

@work933k
Copy link
Contributor Author

Probably not, because we run commands in parallel. Probably this would cause issues.

@work933k
Copy link
Contributor Author

work933k commented Nov 3, 2023

Is there maybe any progress to be reported on this issue?

@ghengeveld
Copy link
Member

No progress. It seems you're using the Node API (run from chromatic/node), is that right? I'm open to have run accept log via the options object. I imagine the same would be useful for sessionId and env. Something like this:

const { sessionId = uuid(), env = getEnv(), log = createLogger(), ...extraOptions } = options;

Happy to accept a PR for this. Unfortunately I cannot prioritize picking this up myself right now. I can create an internal ticket for it though.

@work933k
Copy link
Contributor Author

Hi @ghengeveld ,

I provided the PR #897 897

@ghengeveld
Copy link
Member

@work933k Yep, it shipped in 10.8.0. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Tracking: Issue needs confirmation
Projects
None yet
Development

No branches or pull requests

2 participants