dart-code-metrics icon indicating copy to clipboard operation
dart-code-metrics copied to clipboard

[BUG] Gitlab reporter prints logs and JSON instead of JSON alone

Open pwa-tapptic opened this issue 2 years ago • 8 comments

  • Dart code metrics version: 4.19.2, 4.21.3, 5.4.0
  • Dart sdk version: Dart SDK version: 2.17.3 (stable) (Wed Jun 1 11:06:41 2022 +0200) on "macos_x64"

What did you do? Please include the source code example causing the issue. I run flutter pub run dart_code_metrics:metrics lib --reporter=gitlab > gitlab.json based on the documentation here.

This issue is version-specific, looks like it does not depend on any specific configuration.

What did you expect to happen? As it is demonstrated in the documentation I am supposed to get valid JSON report (gitlab.json) with Gitlab style when I use gitlab reporter.

What actually happened? Versions tested:

  • up until 4.19.2 JSON is valid
  • somewhere between 4.19.2 and 4.21.3 file has started containing also logs
  • it still does not work with the newest version 5.4.0 Example bad JSON file:
[2K
⠙ Analyzing...[2K
[2K
⠹ Analyzing 14 file(s)... 0.5s[2K
⠸ Analyzing 14 file(s)... 2.9s[2K
⠼ Analyzing 14 file(s)... 3.2s[2K
⠴ Analyzing 14 file(s)... 3.3s[2K
⠦ Analyzing 14 file(s)... 3.4s[2K
⠧ Analyzing 14 file(s)... 3.5s[2K
⠇ Analyzing 14 file(s)... 3.5s[2K
✔ Analysis is completed. Preparing the results: 3.8s

[{}, {}] <- JSON ISSUES HERE

🆕 Update available! 4.21.3 -> 5.4.0
🆕 Changelog: https://github.com/dart-code-checker/dart-code-metrics/releases/tag/5.4.0

Are you willing to submit a pull request to fix this bug? Unfortunately no.

pwa-tapptic avatar Jan 13 '23 16:01 pwa-tapptic

@pwa-tapptic can you provide gitlab.json ?

dkrutskikh avatar Jan 13 '23 16:01 dkrutskikh

@dkrutskikh it's actually printed as "Example bad JSON file" in my first message in What actually happened? section.

pwa-tapptic avatar Jan 13 '23 16:01 pwa-tapptic

@pwa-tapptic there are 2 options here: either hide progress indication if the reporter is not a console reporter or add an option of the output file name. Is the progress indication valuable to you to see or it can be omitted?

incendial avatar Jan 14 '23 11:01 incendial

@incendial IMO there is also a third option you may consider, namely using STDOUT to print JSON and STDERR to print progress (I believe that git and many other tools do this as well - they distinguish STDOUT and STDERR and print some messages, not necessarily errors onto STDERR).

Given simple script

echo "message to stdout"
1>&2 echo "msg to STDERR"

when the script is ran then only msg to STDERR is printed and result.txt contains the STDOUT

$ ./script.sh > result.txt
msg to STDERR
$ cat result.txt 
message to stdout

pwa-tapptic avatar Jan 16 '23 07:01 pwa-tapptic

Hmm, interesting suggestion, actually, thank you!

incendial avatar Jan 16 '23 07:01 incendial

Now I'm curious how many systems treat anything printed to stderr as an exit. So, this might introduce a breaking change

incendial avatar Jan 16 '23 07:01 incendial

@pwa-tapptic so a workaround would be to pass --no-congratulate as it should suppress all the messages, while we're fixing the issue

incendial avatar Jan 16 '23 07:01 incendial

I can confirm that adding --no-congratulate at the end of the command works as a workaround 👍

As for the stderr and exiting - it's something to be checked but AFAIK those are two separate things and play separate roles in "CLI app contract", so to speak. You may print plenty of stderr messages (even pure errors), but still exit with 0. You may also print nothing but exit with 1. There are some specific cases where you may have scripts that "listen to" stderr and perform actions based on this, but this is something that needs to be intentionally configured or implemented. Not sure if it could be somehow useful in Flutter and DCM case. I can recall that i.e. in Azure DevOps you may add a script step and enabled an option that will fail the step if something has been printed to stderr. It's an advanced option disabled by default, I can imagine cases when it could be helpful, but I can't imagine how it can be useful in context of Flutter and DCM, especially because it's not that uncommon to use both streams to print messages.

pwa-tapptic avatar Jan 16 '23 08:01 pwa-tapptic