[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests
From: |
Cleber Rosa |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests |
Date: |
Wed, 30 May 2018 18:28:49 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 05/30/2018 05:31 PM, Eduardo Habkost wrote:
> On Wed, May 30, 2018 at 05:03:56PM -0400, Cleber Rosa wrote:
>> On 05/30/2018 04:00 PM, Eduardo Habkost wrote:
> [...]
>> [...] Now, in addition to this very personal, and thus
>> insignificant botheration, it restricts API design. By signaling to
>> users that this is valid:
>>
>> self.vm.add_args(...).launch(...).shutdown()
>>
>> We'd be restricted on the design of, say, migrate(). Even if it sounds
>> sensible to return a boolean indicating migration success, for
>> consistency, it'd require a "return self".
> [...]
>>> I don't see why this would be an argument against making this
>>> possible:
>>>
>>> self.vm.add_args('-nodefaults', '-S').launch()
>>
>> Right, it's *mostly* style. But, it suggests that all methods work
>> similarly and thus restricts API design as mentioned before.
> [...]
>>> They are more readable than the complex method chaining examples
>>> you have shown, but I don't see any argument against:
>>>
>>> self.vm.add_args(...).launch()
>>
>> Right. This one looks nice indeed. *My* issues are related to
>> expectations (all methods support this?), consistency (all methods
>> should support this!) and the design limitations it brings.
>>
>
> These are good points.
>
> I believe it would be a reasonable convention to make methods
> that change QEMU configuration return self (because they are
> often called one after another), but other methods like launch()
> migrate(), and shutdown() don't have to.
>
> So this would work:
>
> self.vm.add_args(...).add_drive(...).launch()
>
> But this doesn't have to:
>
> self.vm.add_args(...).launch().migrate().shutdown()
>
Looks like a sane general rule. Let's revisit this when new add_*()
methods are added.
- Cleber.
- [Qemu-devel] [PATCH v3 0/5] Acceptance/functional tests, Cleber Rosa, 2018/05/29
- [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Cleber Rosa, 2018/05/29
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Stefan Hajnoczi, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Fam Zheng, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Eduardo Habkost, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Cleber Rosa, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Eduardo Habkost, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Cleber Rosa, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Eduardo Habkost, 2018/05/30
- Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests,
Cleber Rosa <=
Re: [Qemu-devel] [PATCH v3 3/5] Acceptance tests: add quick VNC tests, Stefan Hajnoczi, 2018/05/30
[Qemu-devel] [PATCH v3 2/5] scripts/qemu.py: allow adding to the list of extra arguments, Cleber Rosa, 2018/05/29
[Qemu-devel] [PATCH v3 5/5] Acceptance tests: add Linux kernel boot and console checking test, Cleber Rosa, 2018/05/29
[Qemu-devel] [PATCH v3 4/5] scripts/qemu.py: introduce set_console() method, Cleber Rosa, 2018/05/29
[Qemu-devel] [PATCH v3 1/5] Add functional/acceptance tests infrastructure, Cleber Rosa, 2018/05/29