Repository navigation
Conversation
sycl-linux-build.yml and sycl-windows-build.yml only checked out devops/actions, so devops/scripts/ur-test (invoked by the E2E build-only pack step via run-tests/linux(windows)/e2e -> run-tests/ur) was missing, making these jobs fail immediately and upload an incomplete/empty E2E binaries artifact to downstream run-only jobs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
A single hung test (e.g. a GPU driver hang/device-lost) could previously block the whole suite until the job's external timeout-minutes fired, cascading a timeout failure into every later composite-action step (filter-log, report, pack/upload artifacts, cleanup). lit's --timeout kills the hung test itself and lets the run finish normally. This requires psutil, so proactively install it alongside defusedxml.
Previously a failed build-only job would still pack and upload whatever was in build-e2e (even just the bare CMake configure skeleton), and every downstream run-only job would then burn CI time running thousands of tests doomed to fail with 'command not found'. Now both sides check for at least one compiled binary under */Output/ and fail immediately with a clear diagnostic instead of propagating broken state silently.
|
failing test case is not related with this PR |
kekaczma
changed the title
| with: | ||
| sparse-checkout: | | ||
| devops/actions | ||
| devops/scripts |
There was a problem hiding this comment.
i just checked the ci results for this pr and say the new github step summaries which are really nice!
however i noticed one issue, for example:
Passed (1469)
<...> // removed by me
... and 1369 more
IMO we need to print all results here (and similarly for unsuppoted/xfail/etc), I use this often to make sure my test ran
There was a problem hiding this comment.
Glad you like the new summaries! The intention was to have complete, collapsible lists of test names for every status, without truncation. I'll fix the step summary to work that way.
| ref: ${{ github.ref || github.sha }} | ||
| testing_mode: full | ||
| target_devices: native_cpu:cpu | ||
| job_name: ${{ github.job }} |
There was a problem hiding this comment.
can we not do this inside the final workflow/step that actually needs it?
There was a problem hiding this comment.
Good point. I added job_name to generate unique names for the uploaded test log/XML artifacts, but we can derive the job ID from github.job inside the E2E action instead. I'll simplify this and keep an explicit name only where needed to distinguish matrix configurations.
| sycl_compiler: | ||
| required: false | ||
| job_name: | ||
| description: 'Name of the calling job, used to build a unique artifact name' |
There was a problem hiding this comment.
can you explain why we didnt need this until now?
There was a problem hiding this comment.
Previously, E2E used its own log reporting and didn't upload log/XML artifacts through the shared run-tests/ur action. With the migration, we need unique artifact names to avoid collisions between E2E jobs. However, as you pointed out in the other comment, I can derive the job ID from the GitHub context instead of passing it explicitly.
| echo "name=e2e-${safe_name}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Run E2E tests | ||
| uses: ./devops/actions/run-tests/ur |
There was a problem hiding this comment.
can we rename this action? i think sycl calling something with a name like 'run UR tests' is a bit weird
There was a problem hiding this comment.
Agreed, the name is misleading now that the action is also used for SYCL E2E tests. I'll rename it to run-tests/lit, since LIT is the common test runner for both UR and E2E.
| run: | | ||
| mkdir build-e2e | ||
| tar -I 'zstd' -xf e2e_binaries.tar.zst -C build-e2e | ||
| if [ -z "$(find build-e2e -path '*/Output/*' -type f -print -quit)" ]; then |
There was a problem hiding this comment.
can we make a mini-action or script for this instead of copy and pasting it around?
There was a problem hiding this comment.
Agreed, this validation is duplicated in a few places. I'll extract it into a small reusable script rather than another composite action, since it only checks whether the E2E build artifact contains test binaries.
| echo "::endgroup::" | ||
| - name: Report E2E Failures | ||
| if: steps.run_e2e.outcome != 'success' | ||
| safe_name=$(echo "$JOB_NAME" | tr -c '[:alnum:]._-' '-' | sed 's/-\+/-/g; s/^-//; s/-$//') |
There was a problem hiding this comment.
yeah unclear to me whats going on here or why we need to do it now
There was a problem hiding this comment.
This was added to generate unique names for the log/XML artifacts uploaded by the shared test reporting action. Previously, E2E didn't use that upload mechanism. I agree the current approach is unnecessarily complicated, so I'll simplify the naming and handle it closer to where the artifacts are uploaded.
# Conflicts: # .github/workflows/sycl-windows-build.yml
Migrates SYCL E2E test execution and reporting to the shared
ur-testtooling already used by Unified Runtime conformance and adapter-specific tests, replacing the previousninja/awkreporting flow.This brings consistent test statistics, timing information, JUnit XML output, GitHub step summaries, and log/XML artifacts to E2E CI.
Changes
run-tests/uraction, keeping E2E-specific LIT options in their respective actions.setup-ur-testto manage Python dependencies across different CI environments.The changes were validated and refined against CI failures encountered across Linux, Windows, and container-based E2E configurations.