-
Notifications
You must be signed in to change notification settings - Fork 130
[SYCL] Fix failures on tests that use double datatype #1246
Conversation
efb4929
to
907ca10
Compare
907ca10
to
354180c
Compare
In one of other reviews I mentioned that test reporting "Passed" state w/o actually exercising all features it is supposed to test, can be dangerous - e.g. tests run on TGLLP will report "Passed" w/o running fp64 test cases. Alternative is to extract feature-specific test cases (e.g. fp64-specific) into a separate test which would use "REQUIRES: aspect_fp64" and report "UNSUPPORTED" on TGLLP. |
IMO, this is less maintainable and isn't really better. Does anybody even look at UNSUPPORTED on any regular basis? How exactly would that information be used? |
If nobody looks at this, this is another problem. Taking this to the extreme, a product, which passes all the tests, but all tests return "UNSUPPORTED" is not really tested and should not be released. |
Even though (static checks + REQUIRES) solution gives more info on which tests run or which skipped, my final vote was for device.has() because IMO using (static checks + REQUIRES) may cause quick growth of number of tests. |
I think tests should strive to test feature alone, w/o making combination with other features. So I would not expect exponential growth of number of tests (which would actually be even worse - combinatorial explosion - if we want to test combinations). |
In an offline discussion we've agreed to proceed with I've also addressed @AlexeySachkov 's concern in e63b15c. Can somebody please re-review/merge? |
No description provided.