build(ci): Add breeze build and test job #11637
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
dreveman
force-pushed
the
github-ci
branch
20 times, most recently
from
fa3b902 to
6b0eb84
Compare
This adds a flag to control whether breeze test fixtures should be generated at build time or not.
|
@Yuhta This fixes a number of build issues in breeze and introduces CI jobs to build and run all breeze tests on openmp and cuda. I can split this up into multiple PRs if we're not ready to add GPU jobs to CI at this time? |
|
Can we move this into it's own workflow so we can path filer on breeze so we only run it when necessary? Edit: Just saw that this is rather quick but with the gpu runner being on the more expensive side I think it would still make sense to only run this when relevant files are changed (velox/experimenta/breeze/* and maybe CMake/* because you use velox_resolve_dependency?) |
This builds and runs all Breeze unit tests using OpenMP.
This builds and runs all Breeze tests for CUDA.
Makes sense. Please take a look at latest version of the PR where I try to do that. |
|
@assignUser How does the latest version of this look to you? |
|
Ah, one minor thing we could exclude breeze from the other builds trigger but that can be handled in a follow up. |
@assignUser That would be nice. We just need to make sure it's included in wave builds that use breeze. That's limited to a simple integration test today but is expected to increase in the near future. Maybe we exclude everything in breeze except the files that make up the header library. @Yuhta wdyt? |
|
@dreveman Yes the breeze tests should only be run when breeze itself is changed. The integration with wave can be run when wave is changed though. |
Yuhta
added
the
ready-to-merge
Ok, I'll create a PR for only running breeze tests when changing breeze code. My only concern is that if wave is using breeze and we don't run wave tests when making breeze changes then it will be pretty easy to break that integration. |
|
@dreveman That's a good point, we should run breeze + wave tests when breeze is changed |
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@xiaoxmeng merged this pull request in 0d572d2. |
Summary: Pull Request resolved: facebookincubator#11637 Reviewed By: Yuhta Differential Revision: D66560387 Pulled By: xiaoxmeng fbshipit-source-id: b82a544d4f6a2e37abaa5bcc9dba4af4e7cba6c6
No description provided.