-
Notifications
You must be signed in to change notification settings - Fork 124
[cts] Add adapters ignore lists for conformance tests #823
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
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.
Can you please add some info in commit message? Explain a little why it's needed 😉
@@ -0,0 +1 @@ | |||
urContextCreateWithNativeHandleTest.Success/NVIDIA_CUDA_BACKEND___NVIDIA_GeForce_RTX_3060_ |
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.
{{.*}} for device names? Same everywhere else.
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.
Done.
@wlemkows, please use my latest workflow commit from: https://github.com/lukaszstolarczuk/unified-runtime/commits/adapters-hw-ci |
9a69c46
to
2631607
Compare
Done. |
Done. |
@@ -0,0 +1,31 @@ | |||
#! /usr/bin/env python3 |
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.
For now it's fine, but eventually, I'd like to see this script somehow merged with #810. They fulfill a similar role...
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.
Sure, we can plan such changes in the future.
test/conformance/cts_exe.py
Outdated
args = parser.parse_args() | ||
result = subprocess.Popen([args.test_command, '--gtest_brief=1'], stdout = subprocess.PIPE, text = True) # nosec B603 | ||
|
||
while 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.
This could be simplified to sth like:
for line in result.stdout.readlines():
...
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.
Done.
test/conformance/cts_exe.py
Outdated
if (test_case[-1] == ","): | ||
test_case = test_case[:len(test_case)-1] | ||
print(test_case) | ||
exit(0) |
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.
Possibly this exit is redundant, the script should exit with zero code at this point.
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.
Done.
print("is: " + input_line) | ||
print("expected: " + match_line.strip()) | ||
sys.exit(1) | ||
if optional_line is 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.
As I understand, this makes optional lines hard to check for - they can contain literally anything and the test can pass. Can we make opt lines required given a certain test scenario or setup?
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.
That would require us to somehow make the lines "conditional", so something like {{skip if compiler==clang}}
. While that might be useful, it's imho out of scope for this work. And no, optional lines can't be anything - this continue
here will only move forward the match file, not the input file, so if the line doesn't match either the optional line or the next one, it will fail.
test/conformance/cts_exe.py
Outdated
if pat.search(line): | ||
test_case = line.split(" ")[5] | ||
if (test_case[-1] == ","): | ||
test_case = test_case[:len(test_case)-1] |
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.
Try simpler test_case.rstrip(',')
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.
Done.
test/conformance/cts_exe.py
Outdated
test_case = test_case[:len(test_case)-1] | ||
print(test_case) | ||
|
||
output = result.communicate() |
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.
Missing output print?
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.
At this point I do not intend to use this variable, I just want to call the communicate function. Removed.
This change enables conformance tests on HW with adapters. Tests that currently do not pass are ignored, but in the future, when they are all fixed, tests should pass without using this functionality.
Uh oh!
There was an error while loading. Please reload this page.