CI Enhancements

[qarmin]

With good CI, a lot of bugs and regression can be caught and also things like bad formatting

• Check formatting in code by cargo fmt
• Unit tests(are in CI, but seems that are missing in code)
• Integration tests
• Build Bevy also on Windows and MacOS (add build windows block to github actions ci #129)
• Cache for faster building (https://github.com/godotengine/godot/blob/f568cede8d8a4a3f7051afd2ae8502deb09bc157/.github/workflows/linux_builds.yml#L30-L40) (Separate CI jobs, add job caching #357)
• Benchmarks and performance regression testing
• Testing project for regression(panics etc.) - project should use as much as possible elements(This is not available now, since there isn't exists for now any tool that provide software rendering of Vulkan apps)
• Sanitizers support(ASAN, LSAN, TSAN, MSAN) - Bevy use some amount of unsafe code which may be invalid, but also some valid code may cause some leaks or bugs - LeakSanitizer, ThreadSanitizer, AddressSanitizer and MemorySanitizer support rust-lang/rust#38699
  I think, that same project from above should be running but this time with sanitizers support.

[cart]

These are all great ideas!

• unit tests: we do have tests. not as many as i would like, but make sure you're running cargo test --workspace to run everything.
• cache: i had the cache wired up in the past, but due to the large size of build artifacts, the save/restore steps ended up being more expensive than just rebuilding.

Testing project for regression(panics etc.) - project should use as much as possible elements

could you elaborate on this?

[multun]

Can you add some merge queue management to the list, like bors?

[cart]

yup that is actually priority 1 as far as CI is concerned. right now we dont have strong safety guarantees that a build is clean. we really need to fix that asap.

[multun]

I'd also remove caching and regression testing from the list, it doesn't seem yet worth it, the build isn't big nor long enough, and regressions not a big deal until things are a bit more settled

[erlend-sh]

Looking at Amethyst’s recent CI migration to GitHub actions would probably be informative here.

I dunno what’s the best place to look to get the full picture of how it works, but @CleanCut might.

[AngelOnFira]

Hey @qarmin, I come from the Veloren project, and I've done a lot of work on CI in Rust. After taking a look through the current CI, there are a few things I might suggest changing/adding. None of them need to be used, but hopefully it might include something of value 😄

Change:

• Currently, there is only one build job. It might make sense to split into multiple jobs, so that each can run in parallel. The separate jobs might be:
  • cargo check
  • cargo fmt
  • cargo clippy

Add:

• Some other tasks that we do that might be of use
  • Build docs from cargo doc (this might already be done if bevy is on crates.io)
  • cargo audit for security checks

[duncanrhamill]

Hey not sure if this is the right place but would also be nice to have a build status tag in the readme, since the book actually suggests using the git dependency and it would be nice to see if that's immediately going to break a build. Maybe a "last commit that passed" tag would be helpful too?

[AngelOnFira]

@duncanrhamill Good idea, I added that in #325

[AngelOnFira]

@cart in the CI workflow, I see this:

  # NOTE: temporarily disabled because on-push currently uses up too many resources
  # push:
  #   branches: [master]

Since the repo is now public, you get unlimited Action minutes, and up to 20 concurrent jobs. I believe this is enough to allow running on every push?

[CleanCut]

Since the repo is now public, you get unlimited Action minutes, and up to 20 concurrent jobs. I believe this is enough to allow running on every push?

That's correct, public repositories get unlimited minutes. I wrote a comment to this effect several days ago, but must have forgotten to hit the submit button. 😊

[cart]

Well thats fantastic news. Thanks for the heads up. Time to heat up some datacenters 😈

[cart]

Hmm I'm still seeing a 2000 minute limit for the Bevy Organization.

[AngelOnFira]

I think that just includes any minutes that are from repos that aren't public. To test it, it might be worth seeing if it increases past 86 once there is another push to master. But there have also been so many CI jobs running, that I don't imagine it could only be 86 minutes. It's not eating into my personal minutes when I make a PR, so that should mean that forks only use CI minutes at the expense of the org (and not at all in the case of public repos) 💯

[CleanCut]

Yes, you still have a limit for private repos. bevyengine/bevy is a public repo, so it is not subject to those restrictions.