[igt-dev] [Intel-gfx] [PATCH i-g-t 1/8] tests/i915/gem_exec_capture: Remove pointless assert

[Tvrtko Ursulin]

On 03/11/2021 18:44, John Harrison wrote:
> On 11/3/2021 06:50, Tvrtko Ursulin wrote:
>> On 22/10/2021 00:40, John.C.Harrison at Intel.com wrote:
>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>
>>> The 'many' test ended with an 'assert(count)', presumably meaning to
>>> ensure that some objects were actually captured. However, 'count' is
>>> the number of objects created not how many were captured. Plus, there
>>> is already a 'require(count > 1)' at the start and count is invarient
>>> so the final assert is basically pointless.
>>>
>>> General concensus appears to be that the test should not fail
>>> irrespective of how many blobs are captured as low memory situations
>>> could cause the capture to be abbreviated. So just remove the
>>> pointless assert completely.
>>
>> Hm the test appears to be using intel_get_avail_ram_mb() to size the 
>> working set. Suggesting problems with low memory situations should not 
>> apply unless bugs. In which case would a better fix be improving the 
>> sizing logic and changing the assert to igt_assert(blobs)?
> After fixing the sysfs read code to cope with large files, I don't ever 
> see abbreviated captures any more. However, other reviewers objected to 
> asserting anything at all about the final count (whether full size, zero 
> or whatever) on the grounds that low memory issues *might* still occur. 
> And some in quite blunt language as I recall. If you think different, 
> feel free to start your own patch set.

Do you have a link so I can understand the discussion? Because from the 
top of my head I can't imagine what were the objections, I mean what is 
the point of keeping the test but not asserting at the end at least 
something was captured?

Regards,

Tvrtko