-
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
Conversation
Also changed default install directory to avoid ugly copy.
@@ -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 |
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 are e2e_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 anyone
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.
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 to sycl-linux-run-tests.yml
and it's easier if the names are defined here vs defaulted inside the sycl-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
- 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 comment
The 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 build
step one something or some workflow global var or something
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 don't see a good way to do that... The fact that we have any tests built in the sycl-*-build.yml
is kind of a hack to improve CI performance as we avoid extra job/upload/download by building the tests in the same job/on the same runner. That hack in itself is causing this weirdness.
is covered by #15207. |
Also changed default install directory to avoid ugly copy.