-
Notifications
You must be signed in to change notification settings - Fork 794
[SYCL][E2E] Add functionality to split build and run of e2e tests #16016
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 7 commits
7432d1c
be37e41
be6805f
10d2c4c
4eb0d31
0ed6008
dd77b6d
364994d
29048da
b8e6cfc
0f19c31
97369f7
70c2555
1387eed
dfcbcd1
b40fcf3
b8b1b1e
2811890
ef98efd
4bd4857
1f393c0
5fcb771
f3fc08f
f1b98c3
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 |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| // using --offload-compress without zstd should throw an error. | ||
| // REQUIRES: !zstd | ||
| // REQUIRES: unsplit-mode | ||
| // RUN: not %{build} %O0 -g --offload-compress %S/Inputs/single_kernel.cpp -o %t_compress.out 2>&1 | FileCheck %s | ||
| // CHECK: '--offload-compress' option is specified but zstd is not available. The device image will not be compressed. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| // REQUIRES: windows | ||
| // REQUIRES: unsplit-mode | ||
|
|
||
| // TODO: Add hypotf case back when the missing symbol is fixed. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
| // RUN: %{build} -o %t.out | ||
| // RUN: %{run} %t.out | ||
|
|
||
| // XFAIL:* | ||
| // XFAIL: run-mode | ||
|
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. sorry if i asked this before but i wonder if we can implement this inside the python scripts instead of requiring test changes 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. These changes are to differentiate between tests that fail when compiling vs tests that fail when running. These tests originally are able to build so marking as 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 want (very strongly) to start with explicit tests markup first before we introduce any heuristics in the lit logic, especially if that's really heuristics and not some bullet-proof correct logic. 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 guess im just worried this feature will slow down/confuse developers writing tests, im willing to proceed with this as-is and see what happens, but we should be ready to revert/fix the problem fast if some issue comes up 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.
For now since there is no automatic running on CI enabled, we'll just have to monitor the failures manually. However once this is integrated into the post/pre commit I think it should be pretty straightforward to figure out the appropriate action given the ci's behaviour:
|
||
|
|
||
| #include "../common.hpp" | ||
|
|
||
|
|
||
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.
nit: we might want something more descriptive here, if possible
UNSUPPORTED: split-test-build-and-runor something like thatUh oh!
There was an error while loading. Please reload this page.
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.
Will address this comment last to leave open to bike-shedding. Would something like
unsplit-test-modework? orunsplit-testing, should the other features (build-mode,run-mode) follow suit? or are they clear enough.I don't think i'd want to put the word "split" in this particular feature, since it is exactly what it isn't.
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.
yeah of course i dont want to start bikeshedding,
REQUIRES: unsplit-test-modeis fine with meThere 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.
Won't it be clearer to have something like
only-build-test,only-run-test,build-and-run-testinstead ofbuild-mode,run-mode, andunsplit-test-mode?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.
The
run-modefeature is useful because it allows us to have common behaviour between the run-only and unsplit modes, if we didnt have it we would have to mark tests that xfail at run-time as both XFAIL ononly-run-testandbuild-and-run-test.The
build-modefeature I could not think of a valid use for, since if we have an xfail at build time we would always also want to mark it as an xfail at run-time since we wouldn't have an executable, also a test in the e2e folder that usesREQUIRES: build-modeseems a bit suspicious.I added something like
build-only/run-only/unsplitas a variable within the python scripts, but not as a feature since their usefulness in that context is unclear to me. Rather I'm using them to clean up if statements in the python scripts where we want to check what split-mode we are in.