clar: correctly account for "data" suites when counting#5168
Conversation
Failing to do that makes clar miss the last of the suites, as all duplicated "data" would have not been accounted for.
|
(Filed upstream: clar-test/clar#82) |
pks-t
left a comment
There was a problem hiding this comment.
Yup, agreed. Thanks for cleaning up after myself!
|
No problem. I got quite confused by the "no such testsuite z::zz" messages in I think the filtering would also gain to be improved — right now the filter syntax with "data labels" looks like this: Would changing the syntax to something like (Just my 2c, my current pet clar peeve is skip reasons so it might get implemented first 😉). |
|
Yeah, I just stumbled over the exact same issue. Didn't realize at first that we in fact stopped executing those tests altogether! I'm certainly open to changing how the syntax looks like, and filtering by data label would certainly be useful. |
|
But wait, I'm already using your PR but it's still wrong. Are we skipping over modules which have no initializers at all now? |
|
In fact it works correctly without this PR :/ |
|
|
||
| def suite_count(self): | ||
| return len(self.modules) | ||
| return sum(len(module.initializers) for module in self.modules.values()) |
There was a problem hiding this comment.
Are we skipping over modules which have no initializers at all now?
Sounds like it does, yes. min(☝️, 1) then ?
There was a problem hiding this comment.
I have no such test suites in libssh2, which explains why I missed it 😞.
There was a problem hiding this comment.
Yeah, I tried the same locally, but in fact this didn't fix the issue for me. Didn't dig deeper yet, though
There was a problem hiding this comment.
Oops. Because we both had the same issue of using min instead of max. See #5175 for a fix
Failing to do that makes clar miss the last of the suites, as all duplicated "data" would have not been accounted for.