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

Share Go's test cache in CI between jobs to reduce latency for getting the results #1229

Closed
p0mvn opened this issue Apr 8, 2022 · 5 comments · Fixed by #1622
Closed

Share Go's test cache in CI between jobs to reduce latency for getting the results #1229

p0mvn opened this issue Apr 8, 2022 · 5 comments · Fixed by #1622

Comments

@p0mvn
Copy link
Member

p0mvn commented Apr 8, 2022

Context

We would like to investigate and implement, if possible, the solution for sharing Go's test cache between test CI jobs to reduce the number of redundant runs

Initial Investigation

Risks

Acceptance Criteria

  • investigate the solution and alternatives
  • implement reusable test cache
  • the solution is stable
    • it reduces the number of test runs across jobs
    • it runs the tests when a change was made in a package
@ValarDragon
Copy link
Member

ValarDragon commented May 25, 2022

This PR caches the entire go build directory, I don't know if the folder pointed to contains test cache on the CI system: #1583

On inspection of jobs in that PR, tests weren't cached on repeated runs of the same logic. If we see test runs speedup dramatically below 3 minutes, that'd be a good sign caching of tests is happening. The default go test cache location does appear to be getting cached in that PR though. Perhaps the -coverprofile forces re-running of all tests, similar to count=1? https://github.com/osmosis-labs/osmosis/blob/main/Makefile#L233-L234

Though tbh, if this stays at/under 3 minutes, I think its fine. Will get sped up further when we reduce the overhead of CLI tests.

@ValarDragon
Copy link
Member

Ah indeed, golang/go#23565

coverprofile does not get cached with tests at the moment, hence tests are not getting cached. So the decision matrix for this issue is then:

  • Keep coverprofile, close this issue, accept no test caching
  • Remove coverprofile, go tests will be cached / execute extremely fast

I don't think I ever look at codecov comments on PRs / coverprofile personally, but I'm also fine with the current CI time / further speedups by eliminating more CI tests. How do others feel?

@alexanderbez
Copy link
Contributor

I personally don't either, but it can be good to know how much coverage we have at any given moment. I'm in favor of caching, i.e. removing coverprofile

@ValarDragon
Copy link
Member

Sweet, I'm down to remove coverprofile as well

@ValarDragon
Copy link
Member

Ah the other benefit of removing this: No more noise notifications from codecov on your PRs 😂

Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants