Skip to main content

code review for added interview question

3 replies [Last post]
Anonymous

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Allen Wang

Hi Alexander,

I believe CLDC/MIDP does not need to use DTFInterview. DTFInterview is
mainly targeted for CDC based TCKs which may need to start passive agent
on another host. In fact, CommInterview is all it needs.

I took a look at VmInterview. In my opinion, it is not a good candidate
as a base class for CDC based interviews. There are a lot of APIs and
instance variables that are not relavant to CDC based technologies. It
uses "mode" for control flow. This forces the base interview class to be
aware of things that it shouldn't know and makes it difficult to add new
functionality. The base classe has to be updated frequently for new
"mode". We experienced the same problem for TestSuite classes and had to
change it to service based structure.

I think it is good to leave the DTFInterview and other interviews as
standalone building blocks so that client code can easily uses them and
control them the way it wants.

Thanks,
Allen

Alexander Alexeev wrote:

> Hi Allen,
>
> The creation of CommInterview inside DTFInterview lead to two
> instances of
> CommInterview in CLDC/MIDP mode (one from CldcBaseCommInterview and
> another from
> DTFInterview). It will lead to doubling questions in the interview and
> I'm not
> sure it will work even.
> My strong opinion that *ALL* TCKs should use the same base interviews
> including
> VmInterview. This allow to reuse common questions for both modes and
> to create
> all interviews from framework side. BTW as I know there are no
> CDC-based TCK use
> me framework currently (except AGUI but it use VmInterview). All TCKs for
> CDC/CLDC optional packages use MidpTckBaseInterview and VmInterview.
>
> Thanks,
> Alexander
>
> Allen Wang wrote:
>
>> Hi Alexander,
>>
>> Please see my answers below:
>>
>> meframework@mobileandembedded.org wrote:
>>
>>> Hi Allen,
>>>
>>> as was suggested by Vladimir I've started to review your interview
>>> changes and have following comments.
>>>
>>> 1. Why to move CommInterview constants to separate class? Seems they
>>> only make sense in CommInterview and so more appropriate to hold
>>> them there.
>>
>> To create an instance of DTFInterview, you need to pass in an integer
>> argument to indicate the mode of interview (CDC or CLDC). This
>> argument is required to create CommInterview inside DTFInterview.
>> Because reusing CommInterview is an implementation detail, it would
>> be wrong to expose the constant values in CommInterview and require
>> client code to reference it. Therefore, these constant values are
>> moved to a public class.
>>
>>> 2. Is it really needed to export framework location in DTF
>>> interview? It's exported in the MidpTckBaseInterview already. FYI
>>> the method returns framework location will be created in the
>>> J2meTckFrameworkInfo class in the upcoming changes for the enabling
>>> separate framework installation.
>>>
>>>
>> OK. I will remove the code to export the framework location.
>>
>>> 3. I've not seen any access points to the new interviews. Is it
>>> intentional and TCK's specific interview should create them by
>>> itself? I think it possible to detect that TCK need a DTF or
>>> sigtest interview and create them from VmInterview.
>>>
>>>
>> No CDC based TCKs use VmInterview. So I think it is best to have the
>> TCKs create these interviews.
>>
>> Thanks,
>> Allen
>>
>>> Thanks,
>>> Alexander
>>> [Message sent by forum member 'skavas' (skavas)]
>>>
>>> http://forums.java.net/jive/thread.jspa?messageID=204270
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
>>> For additional commands, e-mail: meframework-help@cqme.dev.java.net
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
>> For additional commands, e-mail: meframework-help@cqme.dev.java.net
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

skavas
Offline
Joined: 2006-10-04
Points: 0

Hi Allen,

OK, lets don't touch VmInterview.

Does CommInterview make sense for CDC-stack TCKs without DTF (as for the CLDC-stack)? If yes then it possible to move creation of CommInterview outside DTFInterview in the upper level interview. This allow to reuse DTFInterview in the MIDP mode also. If not then CommInterview in the DTFInterview should be created in CDC mode only.

And I propose to split constants in two classes with appropriate names: one class for transports' constants and another for modes' constants.

Thanks,
Alexander

Allen Wang

Hi Alexander,

>Does CommInterview make sense for CDC-stack TCKs without DTF (as for the CLDC-stack)? If yes then it possible to move creation of CommInterview outside DTFInterview in the upper level interview. This allow to reuse DTFInterview in the MIDP mode also. If not then CommInterview in the DTFInterview should be created in CDC mode only.
>
>
I couldn't think of a CDC stack TCK which may need CommInterview only.
Even if there is one, it can just reuse CommInterview without having to
touch DTFInterview, right? We can see DTFInterview as a wrapper around
CommInterview with a few more optional questions related to DTF for
added convenience for TCKs which have distributed tests.

Right now, I don't think MIDP TCK will use DTFInterview because it does
not care about passive agent. The only benefit for MIDP TCK to use
DTFInterview is that it could reuse the questions for JavaTest host IP
address and remote verboseness for free, which is not a big deal. Also,
DTFInterview exports variables that may have different names in MIDP TCK
interview, like passiveHost and passivePort. So I would say DTFInterview
is not ready (or worth) to be shared with MIDP TCK interview.

In the spirit of "when in doubt, leave it out", I agree that we should
just hard code the CDC mode when creating CommInterview and remove the
argument "mode" from the constructors. We can always add support for
CLDC stack TCK in DTFInterview if it is required later. What do you think?

>And I propose to split constants in two classes with appropriate names: one class for transports' constants and another for modes' constants.
>
>
I don't see any significant benfit for doing this. Why do we need to
introduce an extra class for only two constants?

Thanks,
Allen

meframework@mobileandembedded.org wrote:

>Hi Allen,
>
>OK, lets don't touch VmInterview.
>
>Does CommInterview make sense for CDC-stack TCKs without DTF (as for the CLDC-stack)? If yes then it possible to move creation of CommInterview outside DTFInterview in the upper level interview. This allow to reuse DTFInterview in the MIDP mode also. If not then CommInterview in the DTFInterview should be created in CDC mode only.
>
>And I propose to split constants in two classes with appropriate names: one class for transports' constants and another for modes' constants.
>
>Thanks,
>Alexander
>[Message sent by forum member 'skavas' (skavas)]
>
>http://forums.java.net/jive/thread.jspa?messageID=204836
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
>For additional commands, e-mail: meframework-help@cqme.dev.java.net
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net