Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

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

Open
pwa-tapptic opened this issue Jan 13, 2023 · 8 comments
Open

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

pwa-tapptic opened this issue Jan 13, 2023 · 8 comments
Assignees
Labels
type: bug Something isn't working

Comments

@pwa-tapptic
Copy link

  • 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.

@dkrutskikh
Copy link
Member

@pwa-tapptic can you provide gitlab.json ?

@pwa-tapptic
Copy link
Author

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

@incendial incendial added type: bug Something isn't working in progress labels Jan 13, 2023
@incendial
Copy link
Member

@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?

@pwa-tapptic
Copy link
Author

@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

@incendial
Copy link
Member

Hmm, interesting suggestion, actually, thank you!

@incendial
Copy link
Member

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

@incendial
Copy link
Member

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

@pwa-tapptic
Copy link
Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants