John Snow <js...@redhat.com> writes:
> On 3/23/21 11:44 AM, Markus Armbruster wrote:
>> John Snow <js...@redhat.com> writes:
>>
>>> On 3/23/21 5:40 AM, Markus Armbruster wrote:
>>>> Member name 'u' and names starting with 'has-' or 'has_' are reserved
>>>> for the generator. check_type() enforces this, covered by tests
>>>> reserved-member-u and reserved-member-has.
>>>> These tests neglect to cover optional members, where the name starts
>>>> with '*'. Tweak reserved-member-u to fix that.
>>>> This demonstrates the reserved member name check is broken for
>>>> optional members. The next commit will fix it.
>>>>
>>>
>>> The test without an optional member goes away. Do we lose coverage?
>>> (Do we care?)
>> Up to a point :) We do try to cover all failure modes, just not in
>> all
>> contexts.
>> The test is about this error:
>> if c_name(key, False) == 'u' or c_name(key,
>> False).startswith('has_'):
>> raise QAPISemError(info, "%s uses reserved name" % key_source)
>> Full matrix: (is "u", starts with "has_") x (optional, not
>> optional).
>> Instead of covering all four cases, we cover two: non-optional "u"
>> (reserved-member-u) and non-optional "has-" (reserved-member-has).
>> The patch flips the former to optional. The latter still covers
>> non-optional.
>> Good enough, I think.
>>
>
> Relies a tiny bit on knowing these two reserved name checks are
> implemented in the same place. Doubt it'll matter
> practically. Coverage has increased overall.
>
>> Do you feel I should point to reserved-member-has in the commit message?
>>
>
> It'd be for my benefit, but you also already just explained it to me.
Amending the second paragraph:
These tests neglect to cover optional members, where the name starts
with '*'. Tweak reserved-member-u to fix that. Test
reserved-member-has still covers non-optional members.
> Reviewed-by: John Snow <js...@redhat.com>
Thanks!
