-
Notifications
You must be signed in to change notification settings - Fork 18
Enable most of the cbuffer tests for clang #280
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
Conversation
Test failure is the XPASS being resolved in #284 |
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.
General feedback: should these be unsupported or XFAIL so that we know when they start working?
# https://github.com/llvm/llvm-project/issues/110722 | ||
# XFAIL: Clang | ||
# https://github.com/llvm/llvm-project/issues/138996 | ||
# UNSUPPORTED: Clang |
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.
Do we want this UNSUPPORTED or XFAIL?
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.
XFAIL. I'll update these.
@@ -1,4 +1,4 @@ | |||
if 'Clang' in config.available_features: | |||
if 'Clang-Vulkan' in config.available_features: | |||
config.unsupported = True |
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.
Should we just be enabling these now?
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.
Well, none of these tests pass under vulkan. I could remove this and add XFAILs to the tests directly if you think that makes more sense.
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.
I don't have a strong feeling, I just want to make sure that we don't somehow lose track and not enable the tests once they do start passing.
These have been working for some time but we never enabled the tests.
2bba62c
to
c8007c1
Compare
pre-checkin CI is in a bit of a rough spot, but the cbuffer tests themselves are passing. |
These have been working for some time but we never enabled the tests.