Feature request: Formatter for Azure DevOps

jmarek-sky commented 1 year ago

Formatter specific to Azure DevOps which would allow to report warnings and errors in a native way (which would surface in the Azure DevOps UI).

Context

We use Azure DevOps as our CI/CD tool and recently started using dependency-cruiser, which I personally think is amazing. Thank you for making it.

The challenge we have is that it's not easy to know what violations there are (if any) when looking at the build summary. In the "best" case scenario the build fails (in case of violations with error severity), the error surfaced to the UI says bash exited with non-zero exit code, and the user needs to go into logs of the specific task to know what the violations were. In the "worst" case there are violations which have warning severity, the build passes, everything is green and the user might not even be aware that there were some warnings.

Expected Behavior

Violations detected by dependency-cruiser are surfaced in the Azure DevOps UI natively.

Current Behavior

Violations are buried inside the logs.

Possible Solution

Similarly to the TeamCity format, it should be possible to offer Azure DevOps format which could leverage the "log issue" task command to surface errors and warnings. Additionally the "complete" task command could be used to set the status of the task/job to "SucceededWithIssues" when there are only violations with warning severity and no errors.

FYI a formatter for eslint exists which leverages the above: eslint-formatter-azure-devops

Considered alternatives

I'm currently implementing an alternative solution using BuildQualityChecks Azure DevOps extension, specifically its Warnings Policy.

It seems like it generally works, but:

The documentation of this extension says it mainly works with issues reported natively, offering option to pickup warnings from logs based on regex is just an addition.
When parsing the logs, we're limited in identifying issue categories, which file it originates from, line #, etc.
The warnings picked up by parsing the logs are not counted towards the reported total number of warnings. This means we end up with confusing messages like The number of selected warnings (57 out of 43) exceeds the threshold (50)!. This can happen if there are 43 warnings reported natively (e.g. by eslint formatter) and additional 14 warnings originating from parsing the output of dependency-cruiser.
We need to have another, custom, task/step in the pipeline on top of the extension itself to set the status of the whole build as "SucceededWithIssues" if warnings exist.

sverweij commented 1 year ago

Hi @jmarek-sky thanks for this suggestion & pointers to the right spot of the documentation.

I've started a PR with a barebones implementation (#821) and will fill out additional functionality in the coming days (log messages and summary on par with ~the err or teamcity automated non-regression tests, bit of documentation).

I've published the first (rather crude) implementation to npmjs as dependency-cruiser@13.1.0-beta-1. Let me know what you think (taking into account the caveat above).

sverweij commented 1 year ago

... additionally I notice azure pipelines have a feature to upload a markdown report. I don't know how to write files in an azure pipeline (not a user...?), but dependency-cruiser can produce markdown with the markdown reporter.

Sample output for dependency-cruiser below this line

Forbidden dependency check - results

:chart_with_upwards_trend: Summary

524 modules 1304 dependencies 2 errors 0 warnings 3 informational 0 ignored

rule	violations	ignored	explanation
:grey_exclamation: utl-module-not-shared-enough	3	0	(sample rule to demo rules based on dependents)
:exclamation: not-unreachable-from-cli	2	0	This module in the src/ tree is not reachable from the cli - and is likely dead wood. Either use it or remove it. If a module is flagged for which it's logical it is not reachable from cli (i.e. a configuration file), add it to the pathNot in the 'to' of this rule.

:fire: All violations

Violations found - click to expand

|violated rule|module|to| |:---|:---|:---| |:exclamation: _not-unreachable-from-cli_|src/graph-utl/consolidate-to-folder.mjs|| |:exclamation: _not-unreachable-from-cli_|src/schema/baseline-violations.schema.mjs|| |:grey_exclamation: _utl-module-not-shared-enough_|src/utl/array-util.mjs|| |:grey_exclamation: _utl-module-not-shared-enough_|src/utl/find-all-files.mjs|| |:grey_exclamation: _utl-module-not-shared-enough_|src/utl/wrap-and-indent.mjs||

dependency-cruiser@13.0.5 / 2023-07-07T20:09:16.628Z

jmarek-sky commented 1 year ago

Hi @sverweij , Thank you for a quick response. I'll give that beta build a try soon.

The markdown report could be useful, but it still, I think, wouldn't surface the warnings on the build summary page and wouldn't affect the overall status of the build. The report would most probably be available only after navigating to a specific tab with reports. I can give it a go as well and report back.

jmarek-sky commented 1 year ago

Unfortunately I wasn't able to try the beta package in our pipeline due to incompatible Node version. Which made me realise that the project uses no longer supported version of Node 🤦🏻 ... (side note). It looks like there's no nvm on our custom build agents to use newer Node version, so it will take me a bit longer to actually try it out in Azure DevOps. It looked good when running locally though 😉 I only left couple of suggestions in comments on the PR.

The Markdown reports generally worked and can provide the details without the need to dig into the logs, but the warnings are still not surfaced in the Summary tab and only visible under the Markdown reports tab (I used this extension to publish MD) See below (redacted) screenshot.

jmarek-sky commented 1 year ago

Just for comparison, for anyone else reading this thread, this would be the ideal representation: which can be achieved with the dedicated formatter (which I wasn't able to test unfortunately 😢 )

sverweij commented 1 year ago

Hi @jmarek-sky thanks for this feedback (and also for the PR comments)!

Regarding the nodejs version that's a pity! - what version is the agent on?

In the mean time I've made the reporter more on par with the teamcity and err ones, added non-regression tests and put the rule names in the code field.

The result is published on npmjs as ~dependency-cruiser@13.1.0-beta-2~ dependency-cruiser@13.1.0-beta-3

jmarek-sky commented 1 year ago

what version is the agent on?

It's on 14.21.3. I'll look at either updating it or adding nvm to the build agent this week, as it's a project risk in general.

jmarek-sky commented 1 year ago

Hi @sverweij In the meantime, I was able to workaround the build agent issue and tested the 13.1.0-beta-3 now. I can confirm it shows the warnings on the build summary page in a native Azure way and the expected format (it doesn't show the line/column numbers, but I guess dependency-cruiser doesn't pick these up in general). Looks good! I just left one more comment on the PR.

sverweij commented 1 year ago

hi @jmarek-sky thanks for this validation! I've seen your PR comment and will address it tomorrow.

sverweij commented 1 year ago

The PR (including an update to address your last comment) is merged and is part of dependency-cruiser@13.1.0 . Thanks for your help @jmarek-sky!

sverweij / dependency-cruiser