-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add validation test about creating pipeline layout with null bind group layout #4076
Conversation
Hi @jzm-intel and @kainino0x , With latest webgpu/types I cannot test PTAL, thanks! |
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.
Optional nits but feel free to land with or without!
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.
Looks good to me, thanks!
@@ -162,3 +163,96 @@ g.test('bind_group_layouts,device_mismatch') | |||
t.device.createPipelineLayout({ bindGroupLayouts: [layout0, layout1] }); | |||
}, mismatched); | |||
}); | |||
|
|||
const MaybeNullBindGroupLayoutTypes = ['Null', 'Undefined', 'Empty', 'NonEmpty'] as const; |
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.
Combinatorially adding undefined
increases the subcase count from 90 to 310, but I think that's a bit too many. This test is not that slow, but our combinatorial explosions in the CTS overall are quite out of hand so I'd like to be more cautious when we're adding new tests.
The null vs undefined thing is a trivial WebIDL check and we really just need one tiny test that makes sure null and undefined behave the same way.
That's a bit of extra work though. Would we get complete coverage by testing only the cases that have exactly one Null
/Undefined
/Empty
? This reduces the whole thing to just 30 subcases without having to write another test.
See #4107
Issue: #4075
Requirements for PR author:
.unimplemented()
./** documented */
and new helper files are found inhelper_index.txt
.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.