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

Fix random error with codecov upload #1666

Merged
merged 4 commits into from
Mar 10, 2023

Conversation

jjbustamante
Copy link
Member

@jjbustamante jjbustamante commented Mar 7, 2023

Summary

This PR is adding the Codecov token when the coverage report must be uploaded, in theory, we don't need the token because it is a public repo, but, it seems that not passing the token to the action generates the error.

note: I think we don't need to create the secret, it seems that only adding the token parameter in the github-action fixes the issue

Output

NA

After

NA

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

NA

Resolves #1665

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
@github-actions github-actions bot added the type/chore Issue that requests non-user facing changes. label Mar 7, 2023
@github-actions github-actions bot added this to the 0.29.0 milestone Mar 7, 2023
@jjbustamante jjbustamante marked this pull request as ready for review March 7, 2023 22:49
@jjbustamante jjbustamante requested review from a team as code owners March 7, 2023 22:49
@jkutner
Copy link
Member

jkutner commented Mar 8, 2023

@jjbustamante do we need to put this token in our 1Password too?

@jjbustamante
Copy link
Member Author

@jjbustamante do we need to put this token in our 1Password too?

If fact, I believe we don't need to create the actual token, only add the parameter to the GitHub action.

@jjbustamante
Copy link
Member Author

jjbustamante commented Mar 8, 2023

The test failure here was:

=== RUN   TestInspectBuildpack/InspectBuilder/failure_cases/buildpack_on_registry/unable_to_fetch_buildpack_from_registry/returns_an_untyped_error
[6127](https://github.com/buildpacks/pack/actions/runs/4364941571/jobs/7632896818#step:11:6128)
    inspect_buildpack_test.go:350: Expected nil: remove C:\Users\Admin\AppData\Local\Temp\inspectBuildpack2897829195\registryCopy\3\fo\example_foo: The process cannot access the file because it is being used by another process.

Again, the same error cleaning the temp folder on windows, (we fixed before ignoring the err, as it is done in the lifecycle), I will check that

@dfreilich
Copy link
Member

If this works, that would be awesome! I was searching for a bit, though, and couldn't see any reference that this would help.

The readme emphasizes For public repositories, no token is needed and doubles down again

Note: This assumes that you've set your Codecov token inside Settings > Secrets as CODECOV_TOKEN. If not, you can get an upload token for your specific repo on codecov.io. Keep in mind that secrets are not available to forks of repositories.

So while it definitely passed in this PR, I'm concerned that it's a placebo effect at the moment, and we'll just be adding extra cruft to the actions. If you could point to some verification that adding this helps, would love to get it in

@dfreilich
Copy link
Member

Based on https://community.codecov.com/t/upload-issues-unable-to-locate-build-via-github-actions-api/3954/20 (linked in codecov/codecov-action#932), it seems like we really should add in a secret in an Secret defined in the repo or the organization (the maintainers can add that). The downside is that the secret won't get passed along for forks, but it seems like that hopefully (🤞 ) shouldn't cause them to fail (for in this PR, we're passing an empty token and it's working, so by the same logic it should work for forks as well), so I'm for adding it, together with the token.

I preemptively added it (to our secret vault as well), so I'll approve this

Copy link
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

@jjbustamante
Copy link
Member Author

Thanks @dfreilich !! Let's see, I am also not 100% sure it is going to work, but 🤞

@jkutner jkutner merged commit 759d672 into main Mar 10, 2023
@jkutner jkutner deleted the bugfix/issue-1665-codecov-failed-to-upload branch March 10, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/chore Issue that requests non-user facing changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI pipeline failed with 'Codecov: Failed to properly upload'
3 participants