Skip to content

Commit 3b89b37

Browse files
committed
[SYCL][E2E] Track amount of XFAILed tests
We have a problem with our approach to `XFAIL`ing failed tests - we don't always submit trackers for those failures, thus simply hiding them from us. Some of tests we `XFAIL`ed have been in that state for years and without a tracker submitted about them, we don't even know that there are issues. This patch introduces a new requirement for marking test as expected to fail: every `XFAIL` directive has to be followed by `XFAIL-TRACKER` which points to intel/llvm issue submitted to analyze the failure and remove `XFAIL` directive. The tracker can be referenced either by URL, or through GH shortcuts `owner/repo#NNNNN`. To ensure that the new requirement is followed, a new test was added which checks amount of improper `XFAIL` directives (i.e. those which are not followed by `XFAIL-TRACKER` directive). Similar approach is expected to be applied later for `UNSUPPORTED` directives, but it will be done as a separate PR.
1 parent 761d45d commit 3b89b37

File tree

4 files changed

+66
-0
lines changed

4 files changed

+66
-0
lines changed

sycl/test-e2e/README.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,23 @@ implementation header files is still in progress and the final set of these
320320
"fine-grained" includes that might be officially documented and suggested for
321321
customers to use isn't determined yet. **Until then, code outside of this project
322322
must keep using `<sycl/sycl.hpp>` provided by the SYCL2020 specification.**
323+
324+
## Marking tests as expected to fail
325+
326+
Every test should be written in a way that it is either passes, or it is skipped
327+
(in case it is not compatible with an environment it was launched in).
328+
329+
If for any reason you find yourself in need to temporary mark test as expected
330+
to fail under certain conditions, you need to submin issue to the repo to
331+
analyze that failure and make test passed or skipped.
332+
333+
Once the issue is created, you can update the test by adding `XFAIL` and
334+
`XFAIL-TRACKER` directive:
335+
```
336+
// GPU driver update caused failure
337+
// XFAIL: level_zero
338+
// XFAIL-TRACKER: intel/llvm#DDDDD
339+
```
340+
341+
If you add `XFAIL` without `XFAIL-TRACKER` directive,
342+
`no-xfail-without-tracker.cpp` test will fail, notifying you about that.

sycl/test-e2e/Sampler/normalized-clampedge-linear-float.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// RUN: %{build} -o %t.out
44
// RUN: %{run} %t.out
55
// XFAIL: hip
6+
// XFAIL-TRACKER: intel/llvm#14732
67

78
// CUDA works with image_channel_type::fp32, but not with any 8-bit per channel
89
// type (such as unorm_int8)

sycl/test-e2e/Sampler/normalized-clampedge-nearest.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// Missing __spirv_ImageWrite, __spirv_SampledImage,
66
// __spirv_ImageSampleExplicitLod on AMD
77
// XFAIL: hip_amd
8+
// XFAIL-TRACKER: intel/llvm#14732
89

910
/*
1011
This file sets up an image, initializes it with data,
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// This test is intended to ensure that we have no trackers marked as XFAIL
2+
// without a tracker information added to a test.
3+
//
4+
// The format we check is:
5+
// XFAIL: lit,features
6+
// XFAIL-TRACKER: intel/llvm issue URL|intel/llvm#NNNNN shortcut
7+
//
8+
// REQUIRES: linux
9+
//
10+
// Explanation of the command:
11+
// - search for all "XFAIL" occurences, display line with match and the next one
12+
// -I, --include to drop binary files and other unrelated files
13+
// - in the result, serach for "XFAIL" again, but invert the result - this
14+
// allows us to get the line *after* XFAIL
15+
// - in those lines, check that XFAIL-TRACKER is present and correct. Once
16+
// again, invert the search to get all "bad" lines
17+
// - make a final count of how many ill-formatted directives there are and
18+
// verify that against the reference
19+
//
20+
// RUN: grep -rI "XFAIL:" %S -A 1 --include=*.c --include=*.cpp \
21+
// RUN: --no-group-separator | \
22+
// RUN: grep -v "XFAIL:" | \
23+
// RUN: grep -Pv "XFAIL-TRACKER:\s+.*intel/llvm.*\d+" | \
24+
// RUN: wc -l | FileCheck %s
25+
//
26+
// The number below is a number of tests which are *impropertly* XFAIL-ed, i.e.
27+
// we either don't have a tracker associated with a failure listed in those
28+
// tests, or it is listed in a wrong format.
29+
// Note: strictly speaking, that is not amount of files, but amount of XFAIL
30+
// directives. If a test contains several XFAIL directives, some of them may be
31+
// valid and other may not.
32+
//
33+
// That number *must not* increase. Any PR which causes this number to group
34+
// should be rejected and it should be updated to either keep the number as-is
35+
// or have it reduced (preferrably, down to zero).
36+
//
37+
// If you see this test failed for your patch, it means that you either
38+
// introduced XFAIL directive to a test improperly, or broke the format of an
39+
// existing XFAIL-ed tests.
40+
// Another possibility (and that is a good option) is that you updated some
41+
// tests to match the requried format and in that case you should just update
42+
// (i.e. reduce) the number below.
43+
//
44+
// CHECK: 176

0 commit comments

Comments
 (0)