supports-color icon indicating copy to clipboard operation
supports-color copied to clipboard

Move test for Azure DevOps environment up

Open josundt opened this issue 3 years ago • 5 comments

I recently contributed with PR #126 to support color (level 1) in Azure DevOps pipelines.

I have now tried latest chalk version including my changes in an Azure DevOps pipeline, but unfortunately it did not work. I then found that new condition in the code from my PR was added too late in the function; a preceding condition terminates the function/returns before hitting my code (see code comment).

I have done some better testing now, and moved my Azure DevOps check higher up, right before the condition that terminated the function. With this change I have verified that it works in Azure DevOps. Sorry for not testing better in the first PR, I assumed that my check would work close to the other CI Platform checks.

I hope you can accept this update, hopefully followed up by a new release of chalk.

And BTW: Related to the ESM only vs. hybrid package strategy topic from my previous PR: I actually salute you for your ESM only strategy to force the node ecosystem towards ESM. Your strategy worked on me, I spent close to one week updating my company's 30+ internal npm packages to ESM only packages, and found a temporary solution to Jest unit tests while waiting for better built-in ESM support.

josundt avatar Dec 03 '21 16:12 josundt

Any chance for this be approved/merged/released?

josundt avatar Dec 07 '21 16:12 josundt

Are you sure that Azure DevOps pipelines does not run in TTY? It could be that it's your script, not Azure that is not TTY.

sindresorhus avatar Dec 09 '21 20:12 sindresorhus

@sindresorhus I actually made a copy of the supports-color script and added a console.log statement right before each of the return statements. Then I ran the script it in an Azure DevOps pipeline to see in the build logs from which condition the function returned.

It turned out that the condition...

(haveStream && !streamIsTTY && forceColor === undefined)

...was true. That's why I needed to move the check before this one.

In Azure DevOps (and probably other Hosted CI platforms), the capabilities of the terminal in which the build scripts are being executed is maybe not as important as the capabilities to convert the special color character sequences from the build logs into the the format expected by the UI that displays the logs. Users never witnesses the actual terminal output, just the logs (or "head" of logs while the build is running).

The Azure DevOps Web UI is capable of correctly displaying the build logs with colors in HTML/CSS.

josundt avatar Dec 09 '21 21:12 josundt

This whole exclusion logic seems very flaky, especially if sequence matters.

This should probably be a long the lines of > https://github.com/chalk/supports-color/blob/90fd683d8eb43434dfc49918f945351035475901/index.js#L112

The best solution would be to add a check for force color/ disable color environment variable in the same way the flag is implemented. This way you can just set it in your ci as needed

korostelevm avatar Jan 15 '22 13:01 korostelevm

Alternatively, open an issue with Microsoft and ask them to implement a proper terminal for their CI pipelines.

Qix- avatar Jan 15 '22 14:01 Qix-