Move test for Azure DevOps environment up #128
Conversation
| if ('TF_BUILD' in env && 'AGENT_NAME' in env) { | ||
| return 1; | ||
| } | ||
|
|
||
| if (haveStream && !streamIsTTY && forceColor === undefined) { |
There was a problem hiding this comment.
This condition evalutes to true in Azure DevOps, I therefore needed to move my check higher up.
|
Any chance for this be approved/merged/released? |
|
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 I actually made a copy of the It turned out that the condition... (haveStream && !streamIsTTY && forceColor === undefined)...was 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. |
|
This whole exclusion logic seems very flaky, especially if sequence matters. This should probably be a long the lines of > Line 112 in 90fd683 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 |
|
Alternatively, open an issue with Microsoft and ask them to implement a proper terminal for their CI pipelines. |
I recently contributed with PR #126 to support color (level 1) in Azure DevOps pipelines.
I have now tried latest
chalkversion 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.