Skip to content
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

[CI] Split E2E/CTS tests into composite action #16739

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Conversation

aelovikov-intel
Copy link
Contributor

Two reasons:

  • Make the workflow more maintainable as we're in the process of adding another option (benchmarks)
  • I want to move E2E build-only from a separate job to a step during sycl-linux-build.yml. My first thought was to use in-tree E2E configuration but then it mismatches how we'd run the tests (in a standalone mode) and we have a bunch of tests using rpath during compilation which can't work with such mismatch. As such, outlining E2E support into a composite action would allow to share code between build/run e2e tests in distinct jobs (build vs matrix run-tests).

Two reasons:
  * Make the workflow more maintainable as we're in the process of
    adding another option (benchmarks)
  * I want to move E2E build-only from a separate job to a step during
    `sycl-linux-build.yml`. My first thought was to use in-tree E2E
    configuration but then it mismatches how we'd run the tests (in a
    standalone mode) and we have a bunch of tests using `rpath` during
    compilation which can't work with such mismatch. As such, outlining
    E2E support into a composite action would allow to share code
    between build/run e2e tests in distinct jobs (build vs matrix
    run-tests).
@aelovikov-intel
Copy link
Contributor Author

aelovikov-intel commented Jan 22, 2025

This is how logs for the failure would look like: https://github.com/intel/llvm/actions/runs/12914371704, a bit worse than before the PR, but not as bad as if the entire workflow was moved to a composite action (note that we did the opposite, composite action -> reusable workflow in #10292).

The reason it's better now than prior to #10292 is because the "common" part of sycl-linux-run-tests.yml is still directly in the workflow, so, e.g., sycl-ls has a dedicated foldable entry.

New Nightly run: https://github.com/intel/llvm/actions/runs/12916846102

@aelovikov-intel aelovikov-intel changed the title [CI] Split E2E tests into composite action [CI] Split E2E/CTS tests into composite action Jan 22, 2025
- name: Upload SYCL-CTS binaries
if: always() && !cancelled() && inputs.cts_testing_mode == 'build-only'
uses: actions/upload-artifact@v4
uses: ./devops/actions/run-tests/e2e
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried uses: ./devops/actions/run-tests/${{inputs.tests_selector}} but it doesn't work: https://github.com/intel/llvm/actions/runs/12898787585

Comment on lines -448 to -450
# This job takes ~100min usually. But sometimes some test isn't
# responding, so the job reaches the 360min limit. Setting a lower one.
timeout-minutes: 150
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't preserve this for whatever weird GHA reasons, hope that's fine for now. We can try to introduce another mitigation later if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was made due to weird machine that was hanging sometimes. No need to re-introduce.

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm assuming all the actual logic is basically a copy-paste

@aelovikov-intel
Copy link
Contributor Author

lgtm assuming all the actual logic is basically a copy-paste

Yes, other than the timeout I commented on. And also removal of now redundant pieces in if: tests_selector == e2e && <condition>

check-spirv failures are unrelated, introduced by a recent docker image update.
win ci is irrelevant as it hasn't been modified. All relevant Linux tasks have finished.

Merging to get some new pre/post-commit tasks post-merge before EOD (and to unblock other work).

@aelovikov-intel aelovikov-intel merged commit 0d09eb1 into sycl Jan 22, 2025
40 of 43 checks passed
@aelovikov-intel aelovikov-intel deleted the split-run-tests branch January 22, 2025 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants