Skip to main content

Please review: fix for issue #141 - MIDP TCK throws Error with "Export Tests" and "Custom Execution Mode" in interview

8 replies [Last post]
Anonymous

Guys,

please review fix for issue #141:
https://cqme.dev.java.net/issues/show_bug.cgi?id=141

http://fisheye4.cenqua.com/changelog/cqme/?cs=1336

See attached images in Issue Tracker for more details.

Thanks,
Dmitri.

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

Reply viewing options

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

Alexander,

Can you review? I don't have time right now.

Thanks!!
--Vladimir

On Mon, Jun 25, 2007 at 04:39:06PM +0400, Dmitri Trounine wrote:
> Guys,
>
> please review fix for issue #141:
> https://cqme.dev.java.net/issues/show_bug.cgi?id=141
>
> http://fisheye4.cenqua.com/changelog/cqme/?cs=1336
>
> See attached images in Issue Tracker for more details.
>
> Thanks,
> Dmitri.
>
> ---------------------------------------------------------------------
> 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

Alexander Alexeev

Dmitri,

overall looks good. Some comments below.
1. Is my understanding right and preverifier and export modes are alternative?
If so then
if (isPreverifierMode() || !isTestExportMode()) {
return qAdvConfig;
} else {
return qSecurity;
}

could be simplified to

if (isTestExportMode()) {
return qSecurity;
} else {
return qAdvConfig;
}

2. Is it true advanced intro question is displayed without any other
advanced section's questions in export mode now? I think it's usability issue
and more correctly to go to qSecurity directly in export mode.

Thanks,
Alexander

Vladimir Sizikov wrote:
> Alexander,
>
> Can you review? I don't have time right now.
>
> Thanks!!
> --Vladimir
>
> On Mon, Jun 25, 2007 at 04:39:06PM +0400, Dmitri Trounine wrote:
>> Guys,
>>
>> please review fix for issue #141:
>> https://cqme.dev.java.net/issues/show_bug.cgi?id=141
>>
>> http://fisheye4.cenqua.com/changelog/cqme/?cs=1336
>>
>> See attached images in Issue Tracker for more details.
>>
>> Thanks,
>> Dmitri.
>>
>> ---------------------------------------------------------------------
>> 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

Dmitri Trounine

Alexander,

thanks for review!

> 1. Is my understanding right and preverifier and export modes are
> alternative?
> If so then
> if (isPreverifierMode() || !isTestExportMode()) {
> return qAdvConfig;
> } else {
> return qSecurity;
> }
>
> could be simplified to
>
> if (isTestExportMode()) {
> return qSecurity;
> } else {
> return qAdvConfig;
> }
>
Right! Thanks for pointing it.
> 2. Is it true advanced intro question is displayed without any other
> advanced section's questions in export mode now? I think it's
> usability issue
> and more correctly to go to qSecurity directly in export mode.
>
Not exactly. qAdvIntro is a hidden question, it's isEnabled() method
always returns false. It doesn't export any value and isn't visible to
user. We need this question, because there are two possible next
question after ExportInterview depending on the answer.

Thanks,
Dmitri.

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

Alexander Alexeev

Dmitri,

I think qAdvIntro is not hidden question and serve to introduce advanced
section. And there are following properties in cldcbasecomm.properties:
CldcBaseCommInterview.advIntro.smry=Custom Execution Mode
CldcBaseCommInterview.advIntro.text=The following question allow to specify
custom execution mode.
Am I right?

Thanks,
Alexander

Dmitri Trounine wrote:
> Alexander,
>
> thanks for review!
>
>
>> 1. Is my understanding right and preverifier and export modes are
>> alternative?
>> If so then
>> if (isPreverifierMode() || !isTestExportMode()) {
>> return qAdvConfig;
>> } else {
>> return qSecurity;
>> }
>>
>> could be simplified to
>>
>> if (isTestExportMode()) {
>> return qSecurity;
>> } else {
>> return qAdvConfig;
>> }
>>
> Right! Thanks for pointing it.
>> 2. Is it true advanced intro question is displayed without any other
>> advanced section's questions in export mode now? I think it's
>> usability issue
>> and more correctly to go to qSecurity directly in export mode.
>>
> Not exactly. qAdvIntro is a hidden question, it's isEnabled() method
> always returns false. It doesn't export any value and isn't visible to
> user. We need this question, because there are two possible next
> question after ExportInterview depending on the answer.
>
> Thanks,
> Dmitri.
>
> ---------------------------------------------------------------------
> 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

Dmitri Trounine

Looks, like initially qAdvItro was used as you described, but actually
qAdvIntri is off (look at this:
https://cqme.dev.java.net/files/documents/5914/60175/version1.gif). I
just reused it for new purpose (and made it hidden).

I don't remember when and why it was removed from the path. Need to see
the history of changes...

Alexander Alexeev wrote:
> Dmitri,
>
> I think qAdvIntro is not hidden question and serve to introduce advanced
> section. And there are following properties in cldcbasecomm.properties:
> CldcBaseCommInterview.advIntro.smry=Custom Execution Mode
> CldcBaseCommInterview.advIntro.text=The following question allow to
> specify custom execution mode.
> Am I right?
>
> Thanks,
> Alexander

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

Alexander Alexeev

Dmitri,

did you try to run interview? I just tried and found the intro question is
displayed in normal and export modes. Looks by the code of interview it's not
displayed only in preverifier mode. Not sure this is intentional - may be just
bug.

Thanks,
Alexander

Dmitri Trounine wrote:
> Looks, like initially qAdvItro was used as you described, but actually
> qAdvIntri is off (look at this:
> https://cqme.dev.java.net/files/documents/5914/60175/version1.gif). I
> just reused it for new purpose (and made it hidden).
>
> I don't remember when and why it was removed from the path. Need to see
> the history of changes...
>
> Alexander Alexeev wrote:
>> Dmitri,
>>
>> I think qAdvIntro is not hidden question and serve to introduce advanced
>> section. And there are following properties in cldcbasecomm.properties:
>> CldcBaseCommInterview.advIntro.smry=Custom Execution Mode
>> CldcBaseCommInterview.advIntro.text=The following question allow to
>> specify custom execution mode.
>> Am I right?
>>
>> Thanks,
>> Alexander
>
> ---------------------------------------------------------------------
> 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

Dmitri Trounine

Alexander,

please, see my last updates:

http://fisheye4.cenqua.com/changelog/cqme/?cs=1340
http://fisheye4.cenqua.com/changelog/cqme/?cs=1341

Thanks,
Dmitri.

Alexander Alexeev wrote:
> Dmitri,
>
> did you try to run interview? I just tried and found the intro
> question is
> displayed in normal and export modes. Looks by the code of interview
> it's not
> displayed only in preverifier mode. Not sure this is intentional - may
> be just
> bug.
>
> Thanks,
> Alexander
>
> Dmitri Trounine wrote:
>> Looks, like initially qAdvItro was used as you described, but
>> actually qAdvIntri is off (look at this:
>> https://cqme.dev.java.net/files/documents/5914/60175/version1.gif). I
>> just reused it for new purpose (and made it hidden).
>>
>> I don't remember when and why it was removed from the path. Need to
>> see the history of changes...
>>
>> Alexander Alexeev wrote:
>>> Dmitri,
>>>
>>> I think qAdvIntro is not hidden question and serve to introduce
>>> advanced
>>> section. And there are following properties in cldcbasecomm.properties:
>>> CldcBaseCommInterview.advIntro.smry=Custom Execution Mode
>>> CldcBaseCommInterview.advIntro.text=The following question allow to
>>> specify custom execution mode.
>>> Am I right?
>>>
>>> Thanks,
>>> Alexander
>>
>> ---------------------------------------------------------------------
>> 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

Alexander Alexeev

Dmitri,

looks good, only small comment - there is no need in second string parameter for
hidden question. And I strongly recommended to check interview in all modes.
After fixing constructor you can commit.

Thanks,
Alexander

Dmitri Trounine wrote:
> Alexander,
>
> please, see my last updates:
>
> http://fisheye4.cenqua.com/changelog/cqme/?cs=1340
> http://fisheye4.cenqua.com/changelog/cqme/?cs=1341
>
> Thanks,
> Dmitri.
>
> Alexander Alexeev wrote:
>> Dmitri,
>>
>> did you try to run interview? I just tried and found the intro
>> question is
>> displayed in normal and export modes. Looks by the code of interview
>> it's not
>> displayed only in preverifier mode. Not sure this is intentional - may
>> be just
>> bug.
>>
>> Thanks,
>> Alexander
>>
>> Dmitri Trounine wrote:
>>> Looks, like initially qAdvItro was used as you described, but
>>> actually qAdvIntri is off (look at this:
>>> https://cqme.dev.java.net/files/documents/5914/60175/version1.gif). I
>>> just reused it for new purpose (and made it hidden).
>>>
>>> I don't remember when and why it was removed from the path. Need to
>>> see the history of changes...
>>>
>>> Alexander Alexeev wrote:
>>>> Dmitri,
>>>>
>>>> I think qAdvIntro is not hidden question and serve to introduce
>>>> advanced
>>>> section. And there are following properties in cldcbasecomm.properties:
>>>> CldcBaseCommInterview.advIntro.smry=Custom Execution Mode
>>>> CldcBaseCommInterview.advIntro.text=The following question allow to
>>>> specify custom execution mode.
>>>> Am I right?
>>>>
>>>> Thanks,
>>>> Alexander
>>>
>>> ---------------------------------------------------------------------
>>> 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
>

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