Flaky static-analysis with invisible warnings
Job #3520323678 failed for 8aae5391:
Node finished running all tasks in 417.773249089 seconds (expected 478)
All static analyses passed successfully with warnings.
There were warnings:
**** bin/rake gettext:lint had the following warning(s):
It's weird. I didn't see anything printed following that. This happened in other jobs as well:
-
https://gitlab.com/gitlab-org/gitlab/-/jobs/3521153003
Node finished running all tasks in 441.836836753 seconds (expected 478) All static analyses passed successfully with warnings. There were warnings: **** bin/rake gitlab:sidekiq:sidekiq_queues_yml:check had the following warning(s):
-
https://gitlab.com/gitlab-org/gitlab/-/jobs/3523276954
Node finished running all tasks in 427.890752726 seconds (expected 478) All static analyses passed successfully with warnings. There were warnings: **** bin/rake gettext:updated_check had the following warning(s):
Looking at the code, I have no clues how this can happen, however, by writing the analyzed steps down, I figured where it might go wrong.
- They're printing
passed successfully with warnings
indicating that this didn't pass:static_analysis.all_success_and_clean?
butstatic_analysis.all_success?
did pass - It means that all the sub-processes were giving successful status (i.e.
0
), yet it's printing something to the captured stderr (which was not printed yet) - Following that, we printed
had the following warning(s)
to stderr, and then we immediately printed the captured stderr to stderr (which was redirected to$warning_file
), and exit the program normally - I had suspected this might have anything to do with buffered stderr, but the program did exit successfully and we also redirected the stderr to a temporary file, which should make sure it's flushed.
- At this point I realized what can go wrong. Those warnings that we supposed to be able to read, were likely got ignored and filtered in
fail_on_warnings
. - However the above messages it's attempting to write into stderr (
had the following warning(s)
) were treated as warnings as well.
@splattael This reminded me that you changed Edited: Memory didn't serve me right: !107464 (diffs, comment 1216117701)warn
to puts
in previous revisions, which made a lot of sense, yet reverted back to puts
I believe because of mixing text between fail_on_warnings
and scripts/static-analysis
, which can be very confusing and misleading.
I am not sure if this can be fixed/tweaked easily
On the other hand, I also have a question about what if the warnings contain multiple blank lines for formatting purpose? Will grep
remove them because blank lines are ignored? If that's the case, it might mess up the formatting, too. Edited: Confirmed no problem: #386693 (comment 1221129946)
I hope we don't need to insert fail_on_warnings
into each sub-processes, but I wonder if it's possible to handle them in one pass.