Re: [PATCH 1/4] Revert "migration: modify test_multifd_tcp_none() to use new QAPI syntax"

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Thu, Apr 11, 2024 at 04:41:16PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Thu, Apr 11, 2024 at 11:31:08PM +0530, Het Gala wrote:
>> >> I just wanted to highlight couple of pointers:
>> >> 1. though we are using 'channels' in the precopy tests for 'migrate' QAPI,
>> >> we
>> >>    use the old uri for 'migrate-incoming' QAPI.
>> >> 2. We do not cover other 'channels' abi, only have tcp path tested.
>> >> 
>> >> So, the TO-DOs could be:
>> >> 1. Omit the 4th patch here, which introduced postcopy qtests with 
>> >> 'channels'
>> >>    interface OR have 'channels' interface with other than tcp transport
>> >>    (file, exec, vsock, etc) so as to cover different code paths.
>> >> 2. Extend channels interface to migrate-incoming QAPI for precopy qtests
>> >
>> > You can see whether Fabiano has anything to say, but what you proposed
>> > looks good to me.
>> 
>> Ok, so what about we convert some of the 'plain' tests into channels to
>> cover all transports?
>> 
>> - tcp: test_multifd_tcp_none  (this one we already did)
>> - file: test_precopy_file
>> - unix: test_precopy_unix_plain
>> - exec: test_analyze_script
>> - fd: test_migrate_precopy_fd_socket
>> 
>> Those^, plus the validate_uri that's already in next should cover
>> everything.
>> 
>> We don't need to do this at once, by the way.
>> 
>> Moreover:
>> 
>> - leave all test strings untouched to preserve bisecting;
>> 
>> - let's not bother adding "channels" and "uri" to the test string
>>   anymore. The channels API should be taken for granted at this point, I
>>   don't expect we start hitting bugs that will require us to run either
>>   foo/uri/plain or foo/channels/plain, so there's not much point in
>>   making the distinction.
>
> Do you mean we can put "uri:" aside?  Maybe I misunderstood..

I mean the test name does not need to specify "channels" vs. "uri"
because that should never be broken to the point that we actually need
to go fetch those tests by name. We'd still have at least 1 test for
each transport with channels and (existing) at least 1 test for each
transport with uri.

>
> The matrix previously was (I think.. when this series posted):
>
>   [tcp, unix, file, exec, fd] x [uri, channels] x [precopy, postcopy]
>
> Drop postcopy as doesn't seem to have any special paths:
>
>   [tcp, unix, file, exec, fd] x [uri, channels]
>
> So logically we should still cover these, right?

Right, I'm just suggesting we convert some tests to use channels, one
for each transport, to test the channels API in full. The rest of the
existing tests as well as future tests need not have a uri (or channel)
variant. Just one of them is enough.