Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove debug output when coverage fails #20733

Merged
merged 7 commits into from
Aug 12, 2022

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Aug 9, 2022

When make coverage fails, it logs megabytes of debug to stdout, which seems to break the drone ui as well as the log output download in drone, presumably because drone limits downloadable log size to 5MB. I think with removal of this print, we should still see any errors that gocovmerge.go logs to stderr, but a few CI runs may be necessary to get it to fail again.

Example failures:

https://drone.gitea.io/go-gitea/gitea/58933/2/18
https://drone.gitea.io/go-gitea/gitea/58928/2/17

@silverwind silverwind added topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Aug 9, 2022
@silverwind
Copy link
Member Author

silverwind commented Aug 9, 2022

Debug output was added in c0f5da3 by @zeripath, but I think as it stands now, it's not useful as long as drone truncates it to 5MB. I find no documented way to increase this 5mb limit, I think drone hardcodes it currently, latest reference I find is harness/harness#2456 (comment).

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 9, 2022
Makefile Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

zeripath commented Aug 9, 2022

If you remove this logging then we have no way of working out why the coverage is failing.

Do we understand why coverage keeps failing at present?

@silverwind
Copy link
Member Author

silverwind commented Aug 9, 2022

Do we understand why coverage keeps failing at present?

No, but maybe a few re-runs of this will reveal the error without all the debug spam. As is stands, the debug output is useless as long as drone truncates it to 5MB.

By the way, what is the point of this coverage task? Is it needed for external tools like CodeCov? Surely those tools can generate coverage stats on their own?

@silverwind
Copy link
Member Author

silverwind commented Aug 9, 2022

I see that these tasks actually uploads to codecov. Still I wonder the value. I think most contributors ignore code coverage and these codecov reports are generally perceived as spam because the percentages are not stable.

I think we should just drop this codecov integration as it introduces unnecessary build failures. Anything that can fail a build randomly should be eliminated, as it just wastes everyone's time 😉.

When coverage fails, it logs megabytes of debug to stdout, which seems
to break the drone ui as well as the log output download in drone,
presumably because of the size. I think with removal of this print, we
should still see any errors created by gocovmerge.go, but a few CI runs
may be necessary to get it to fail again.
Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nobody is interested or actively working with coverage, don't see any justification to have this code.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 12, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 12, 2022
@techknowlogick techknowlogick merged commit 74515d3 into go-gitea:main Aug 12, 2022
@techknowlogick techknowlogick added this to the 1.18.0 milestone Aug 12, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 15, 2022
* giteaofficial/main:
  Remove follow from commits by file (go-gitea#20765)
  [skip ci] Updated translations via Crowdin
  Fix `make watch` for generated files (go-gitea#20794)
  Add missing translation for queue flush workers (go-gitea#20791)
  Update zh-cn translation for Installation from source (go-gitea#20772)
  Improve comment header for Mobile (go-gitea#20781)
  Add myself to MAINTAINERS (go-gitea#20786)
  [skip ci] Updated licenses and gitignores
  Preserve unix socket file (go-gitea#20499)
  Switch Unicode Escaping to a VSCode-like system (go-gitea#19990)
  Fix git.Init for doctor sub-command (go-gitea#20782)
  Remove the translation key website by PR go-gitea#20777 (go-gitea#20779)
  Move the official website link at the footer of gitea (go-gitea#20777)
  Remove useless JS operation for relative time tooltips (go-gitea#20756)
  Remove debug output when coverage fails (go-gitea#20733)
  Slightly reduce exclamation icon size (go-gitea#20753)
@silverwind silverwind deleted the nocovspam branch August 16, 2022 11:17
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 28, 2022
* Remove debug output when coverage fails

When coverage fails, it logs megabytes of debug to stdout, which seems
to break the drone ui as well as the log output download in drone,
presumably because of the size. I think with removal of this print, we
should still see any errors created by gocovmerge.go, but a few CI runs
may be necessary to get it to fail again.

* Update Makefile

* restart ci

* restart ci

* restart ci

* restart ci

Co-authored-by: techknowlogick <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants