[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases



On 04/16/2012 08:03 AM, Osier Yang wrote:
> s/sharing in/sharing among/, but given it's already
> pushed. let's live with it.
> 
> On 2012年04月10日 21:38, Guannan Ren wrote:
>>      sharedmod.py
>> ---
>>   sharedmod.py |   16 ++++++++++++++++
>>   1 files changed, 16 insertions(+), 0 deletions(-)
>>   create mode 100644 sharedmod.py
>>
>> diff --git a/sharedmod.py b/sharedmod.py
>> new file mode 100644
>> index 0000000..8af26d8
>> --- /dev/null
>> +++ b/sharedmod.py
>> @@ -0,0 +1,16 @@
>> +# This is a module for variable sharing across testcases during
>> +# running. You have to import it in each of testcases which want
>> +# to share data. The framwork have already set {'conn': connobj}
>> +# in libvirtobj dictionary for use in testcases.
>> +
>> +# The libvirtobj dictionary is only set and used by framework
> 
> Any checking? I.e could testcase r/w it too?
> 
>> +# in testcases you could use sharedmod.libvirtobj['conn'] to get
>> +# the connection object in libvirt.py, you need not to close it,
> 
> IMHO "need not" should be "should never". How about the
> follow up use of connection if it's already closed, reopen one
> again?
> 
>> +# the framework do it.
> 
> <snip>
> The framwork have already set {'conn': connobj}
>> +# in libvirtobj dictionary for use in testcases.
>> +
> </snip>
> 
> The comment above should resides here.
> 
>> +libvirtobj = {}
>> +
>> +# shared variables for customized use in testcases
>> +# set variable: sharedmod.data['my_test_variable'] = 'test_value'
>> +# check the variable: sharedmod.data.has_key('my_test_variable')
>> +# get the varialbe: sharedmod.data.get('my_test_variable',
>> 'test_variable_default_value')
> 
> From my p.o.v, "libvirtobj" is a confused name, also "data" is
> not enough to tell what the meaning is. How about use
> "framework_shares" and "case_shares" instead?
> 
> And implement get/set/has functions for both "framework_shares"
> and "case_shares", but not access them directly. e.g.
> 
> def case_shares_get(key):
>     pass
> 
> def case_shares_set(key, value):
>     pass
> 
> def case_shares_has(key)
>     pass
> 
> Even perhaps a class is good.
> 
> Osier
> 
> -- 
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

That's getting closer and closer to what I recommended as a redesign for
the whole test-API. Class would be better, tests shouldn't need to
import the mod every time, there should be checking etc. However I feel
the need to make this stable asap and after some release, we can go
ahead with some major redesigns (or at least that's how I understood our
talk about this with Dave few weeks ago).

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]