Re: group for tests that are dangerous for verifiers?

Mark Tinguely
2013-06-24T13:51:01Z

On 06/23/13 18:44, Dave Chinner wrote:
> On Sun, Jun 23, 2013 at 05:57:49PM -0500, Eric Sandeen wrote:
>> On 6/23/13 5:50 PM, Dave Chinner wrote:
>>> On Fri, Jun 21, 2013 at 01:45:46PM -0500, Eric Sandeen wrote:
>>>> On 6/20/13 12:54 PM, Mark Tinguely wrote:
>>>>> Do we need a xfstest verifier dangerous group?
>>>>>
>>>>> xfstest 111 purposely damages inodes. In hindsight it make sense
>>>>> that it asserts when running with verifiers.
>>>>
>>>> But it only asserts on a debug kernel...
>>>
>>> Right, and it has done so for years - blaming verifiers for
>>> triggering the assert failure is simply shooting the messenger.
>>
>> But this test *intentionally* corrupts, right? So it's prudent
>> to not run a test which you *know* will explode if it runs
>> as designed.
>
> Common sense, really.

... and the reason for the open question. Should there be a group
notation that says the verifiers will *correctly* find a real and known
problem on this test.
>
>>>> This isn't the only place where corruption could ASSERT on debug;
>>>> see xlog_recover_add_to_trans() for example.
>>>>
>>>> But if the test intentionally corrupts it and that leads to
>>>> an ASSERT that does seem problematic for anyone testing w/ debug
>>>> enabled.
>>>
>>> Yup, it runs src/itrash.c which corrupts every inode it can find.
>>>
>>> That's the reason this test is not part of the auto group - it's
>>> a test that will cause the system to stop. We've got other tests
>>> that are not part of the auto group for exactly the same reason -
>>> they cause some kind of terminal failure and so aren't candidates
>>> for regression testing.
>>
>> Then maybe just part of the normal dangerous group would be enough.
>
> It will only run from the ioctl group today (bulkstat, I guess), so
> I'd say that adding it to the dangerous group doesn't add any real
> value except documentation. And it's just as easy to remove the
> ASSERT() as it is really unnecessary....
>
>> Except this isn't transient (today) - it's not a case where old kernels
>> may oops, it's where it's *designed* to oops on this test, with a debug
>> kernel.
>>
>> So I guess I could see a debug-dangerous group ;)
>>
>>>> I guess I'd vote for removing the ASSERT unless there's
>>>> some reason it should be there - Dave?
>>>
>>> I'm fine with it being removed - we catch the failure just fine. If
>>> that then makes 111 work as a regression test (i.e. doesn't trigger
>>> the bad-inode bulkstat loop it was designed to test) then perhaps we
>>> can consider making that part of the auto group, too...
>>
>> Removing it sounds like the best option then.
>
> *nod*
>

That works too.

Thanks,

--Mark.