feat(ado-improvement-1): Intercept & adjust logging in scan process #1030
Conversation
| import { hookStream } from './hook-stream'; | ||
| import { Writable } from 'stream'; | ||
|
|
||
| /* eslint-disable @typescript-eslint/no-explicit-any */ |
There was a problem hiding this comment.
Lint isn't a fan of any as a parameter type, but that's what the Node uses. It might be possible to scope these to single lines, but then we just have more of them. Happy to entertain options for other ways to accomplish this without needing the overrides.
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| /* eslint-disable @typescript-eslint/unbound-method */ |
There was a problem hiding this comment.
Lint isn't a fan of any as a parameter type, but that's what the Node uses. It might be possible to scope these to single lines, but then we just have more of them. Happy to entertain options for other ways to accomplish this without needing the overrides.
| }; | ||
|
|
||
| // This class simply records the contents of any _write calls for validation | ||
| class TestStream extends Writable { |
There was a problem hiding this comment.
I tried unsuccessfully to use a Mock here, but was ultimately unable to get things to work. The TestStream class allowed me to compactly mock out the piece that I care about. Please let me know if you have better ideas on how to do this.
| method: (rawData: string, regex?: RegExp) => string; | ||
| }; | ||
|
|
||
| const regexTransformations: RegexTransformation[] = [ |
There was a problem hiding this comment.
The first iteration of this code defaulted to not marking the output as ##[debug], but that would be difficult to maintain. This version reverses that and will prepend `##[debug] unless we find a regex-based reason not to mark it. We'll need to update this when we figure out how we report the final error, but this sets us up for that.
|
|
||
| return prependDebugPrefix(rawData); | ||
| }; | ||
|
|
There was a problem hiding this comment.
This is easy to pull into a separate file if stderr-transformer later needs to support regex
|
|
||
| export const stderrTransformer = (rawData: string): string => { | ||
| if (rawData.startsWith('waitFor is deprecated')) return null; | ||
| if (rawData.startsWith('Some icons were re-registered.')) return null; |
There was a problem hiding this comment.
The icon message just started to appear in my branch. It doesn't appear in main and I don't know where it came from, but it's not a user-actionable message either way
There was a problem hiding this comment.
Looks like this - microsoft/accessibility-insights-web#2597
Details
As called out in #950, our logging output is hard to read because of a lot of noise that is inconsistently formatted. This PR helps to reduce the noise by blocking some of the noise completely, while other output is formatted to make it easier to filter in tools. Much of the logging is coming from transitive dependencies that are difficult to change, so the traditional approach of injecting a Logger into the class doesn't really do what we need. Instead, we intercept the
stdoutandstderroutput streams where the data gets written, and apply a customized transformer to either remove or reformat the output, depending on the specific need.Motivation
Part of ADO improvement feature
Output
Output before these changes--includes waitFor errors, mixes [Trace][info] and INFO as header tags in the stream, and has no ##[debug] header on things written by the extension:
Output after these changes--excludes waitFor errors. Normalizes [Trace][info] and INFO header tags to ##[debug] in the stream, and also adds '##[debug] ' prefix to output from the extension: A later PR will make this smarter about handling errors (Exceptions or otherwise)
Context
Pull request checklist
yarn test)<rootDir>/test-results/unit/coverageyarn precheckin)