-
Notifications
You must be signed in to change notification settings - Fork 798
[CI] Control each kind of pre-built E2E tests by its own input #19645
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,9 @@ jobs: | |
changes: ${{ needs.detect_changes.outputs.filters }} | ||
|
||
toolchain_artifact: sycl_linux_default | ||
e2e_binaries_artifact: sycl_e2e_bin_default | ||
e2e_binaries_artifact: e2e_bin | ||
e2e_binaries_spirv_backend_artifact: e2e_bin_spirv_backend | ||
e2e_binaries_preview_artifact: e2e_bin_preview | ||
|
||
# If a PR changes CUDA adapter, run the build on Ubuntu 22.04 as well. | ||
# Ubuntu 22.04 container has CUDA 12.1 installed while Ubuntu 24.0 image | ||
|
@@ -131,13 +133,13 @@ jobs: | |
image_options: -u 1001 --device=/dev/dri -v /dev/dri/by-path:/dev/dri/by-path --privileged --cap-add SYS_ADMIN | ||
target_devices: level_zero:gpu;opencl:gpu;opencl:cpu | ||
extra_lit_opts: --param spirv-backend=True | ||
e2e_binaries_artifact: sycl_e2e_bin_default_spirv_backend | ||
e2e_binaries_artifact: e2e_bin_spirv_backend | ||
- name: Preview Mode | ||
runner: '["Linux", "gen12"]' | ||
image_options: -u 1001 --device=/dev/dri -v /dev/dri/by-path:/dev/dri/by-path --privileged --cap-add SYS_ADMIN | ||
target_devices: level_zero:gpu;opencl:gpu;opencl:cpu | ||
extra_lit_opts: --param test-preview-mode=True | ||
e2e_binaries_artifact: sycl_e2e_bin_default_preview | ||
e2e_binaries_artifact: e2e_bin_preview | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kinda of a nit maybe but i wonder if we could single-source these names by making it an output of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a good way to do that... The fact that we have any tests built in the |
||
|
||
uses: ./.github/workflows/sycl-linux-run-tests.yml | ||
with: | ||
|
@@ -151,7 +153,7 @@ jobs: | |
toolchain_artifact: ${{ needs.build.outputs.toolchain_artifact }} | ||
toolchain_artifact_filename: ${{ needs.build.outputs.toolchain_artifact_filename }} | ||
toolchain_decompress_command: ${{ needs.build.outputs.toolchain_decompress_command }} | ||
e2e_binaries_artifact: ${{ matrix.e2e_binaries_artifact || 'sycl_e2e_bin_default' }} | ||
e2e_binaries_artifact: ${{ matrix.e2e_binaries_artifact || 'e2e_bin' }} | ||
e2e_testing_mode: 'run-only' | ||
|
||
# Do not install drivers on AMD and CUDA runners. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again not really related but i wonder if we really need to expose as inputs to
sycl-linux-build
, could we just require they aree2e_bin{_spirv_backend, _preview}
? i don't expect these will ever be used by anyone outside of CI testing so the name shouldnt matter to anyoneThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm trying to do is to allow specifying only some kinds of e2e tests vs all-or-nothing (I'm not interested in preview mode when creating a container image for the compatibility testing).
It doesn't make much difference between three
bool
s with default names vs three explicit names. However, we do need to pass the names later on tosycl-linux-run-tests.yml
and it's easier if the names are defined here vs defaulted inside thesycl-linux-build.yml
workflow.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can also think of possibly having two toolchain builds (e.g. libstdc++ and libcxx) in a single task (e.g. Nightly/Weekly). We'd need to separate e2e tests built with either toolchain, so having this name would be handy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it thx