Skip to main content

Please review my fixes for 6567113, 6585764, 6507112

4 replies [Last post]
Anonymous

Please review my proposed fix. This change set should cover:

6567113 add callback API before propagation is started and after
configuration is completed
6585764 Expose API to control properties overriding during Configuration
Propagation
6507112 add Test Pack names to template propagations conflicts and updates

Also this proposal can be base for implementing this one:

6559873 add ability to change captions(title, tabs,...) in configuration
propagation window

Also I extracted few utility methods in InterviewPropagator class which
could be necessary for property questions processing, see below.

The diff should be here
http://fisheye4.cenqua.com/changelog/jtharness/?cs=315

Our fisheye works very slow with extremely delays but I hope changeset
will appear there.

//
// utility functions (offered as external API)
//

/**
* Converts string representation of property question to Properties2
object
* @param str - string representation of property question
* @return coresponding Properties2 object
* @throws IOException
*/
public static Properties2 stringToProperties2(String str) throws
IOException

/**
* Properties2 object to its string representation.
* Used for property question processing
* @param pr - Properties2 object
* @return corresponding string representation
*/
public static String properties2ToString(Properties2 pr)

/**
* Returns is the specified question is properties question
* @param key - question key
* @param interview - InterviewParameters object
* @return true if the specified question is properties question,
owervise false
*/
public static boolean isPropertyQuestion(String key, InterviewParameters
interview)

/**
* Returns is the specified question is properties question
* @param key - question key
* @param allQ - question map
* @return true if the specified question is properties question,
owervise false
*/
public static boolean isPropertyQuestion(String key, Map allQ)

Here is a sample of usign CustomPropagationController:
------------------------------------------------------

CustomPropagationController pc = new CustomPropagationController() {

// 6567113 add callback API before propagation is started and after
configuration is completed
public void notify(EventType evt) {
switch (evt) {
case Start:
System.out.println("STARTED");
break;
case Finish:
System.out.println("FINISHED");
}
}

// 6507112 add Test Pack names to template propagations conflicts
and updates
public String getQuestionText(String key, String defaultText) {
return changedQuestions.getProperty(key, defaultText);
}

// 6585764 Expose API to control properties overriding during
Configuration Propagation
public boolean preprocessData(Properties templateData,
InterviewParameters interview) {
String key = "jck.timeout.timeout";
// get template's value
int tv = Integer.parseInt(templateData.getProperty(key, "4" ));
if (interview.getAllQuestions().containsKey(key)) {
Question q = (Question) interview.getAllQuestions().get(key);
try {
// set timeout as template's * 2
q.setValue("" + tv*2);
} catch (Fault fault) {
fault.printStackTrace();
}
return true;
}
return false;
}

private Properties changedQuestions;

{
// redefine two question texts:

changedQuestions = new Properties();

// default text is "Specify a time factor that is applied to
each test's default timeout.
// For example, specifying "2" doubles the time for each test
(the default is 1):"
changedQuestions.setProperty("jck.timeout.timeout", "Time
factor that is applied to each test's default timeout");

// default text is "Enter the upper bound for the range of TCP
ports used by the tests.
// It should be in the range 1 to 65535, and should be greater
than or equal to the lower bound."
changedQuestions.setProperty("jck.env.runtime.tcp.tcpUpper",
"Upper bound for the range of TCP ports used by the tests");
}

};

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

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
bkurotsu
Offline
Joined: 2004-12-13

Wondering why string conversions for Properties2 is offered as API on InterviewPropagator class? If the original object was from the com.sun.interview.Properties2 class, which not offer that API as public static methods on that object. Seems much better that way, and it makes no logical sense on InterviewPropagator, other than the fact that it is useful in that part of the code today.

Diff for InterviewParameters is pretty hard to read - 90% null change it seems... :(

In the javadoc for CustomPropagationController, I suggest reminding the developer in the API that the string returned from getQuestionText() should be internationalized. And, should the name of this method be getQuestionKeyText()?

And a possible alternative architecture for the classes. You are using the CustomPropagationController as the default implementation as well. Perhaps make an abstract base class, then provide a default implementation class (like DefaultPropagationController).

None of my suggestions affect the core functionality you are providing. And I think you can correctly refactor the code as needed - go ahead and make your updates and integrate into trunk. I'll recheck it after integration.

ersh
Offline
Joined: 2006-10-18

> Wondering why string conversions for Properties2 is
> offered as API on InterviewPropagator class? If the
> original object was from the
> com.sun.interview.Properties2 class, which not offer
> that API as public static methods on that object.
> Seems much better that way, and it makes no logical
> sense on InterviewPropagator, other than the fact
> that it is useful in that part of the code today.

Yes, agree, this is not elegant solution. But there are some reasons for this. I need (and the customer needs it) to process ProperiesQuestion's value not only as solid serialized string but also as set of properties. So I need a way to disassemble, change and assemble back the ProperiesQuestions' values. But this means using com.sun.interview.Properties2 because this class was chosen as serializer/deserilizer for ProperiesQuestion. Sure I can wrap Properties2 to java.util.Properties but I see some lacks, at least performance overhead.

>
> Diff for InterviewParameters is pretty hard to read -
> 90% null change it seems... :(

Fisheye can ignore dummy diffs. Look at the top-right corner, set
Ignore whitespace - all, blank lines, click "apply", enjoy ;-)

>
> In the javadoc for CustomPropagationController, I
> suggest reminding the developer in the API that the
> string returned from getQuestionText() should be
> internationalized. And, should the name of this
> method be getQuestionKeyText()?

I'm sorry I don't understand why it should be getQuestionKeyText()? This is “custom Question.getText()”

>
> And a possible alternative architecture for the
> classes. You are using the
> CustomPropagationController as the default
> implementation as well. Perhaps make an abstract
> base class, then provide a default implementation
> class (like DefaultPropagationController).

This is style question. I did it because I don't want to make things more complex then it needs. And also I'd say such approach is widely used, rememberer well known ContextManager

>
>
> None of my suggestions affect the core functionality
> you are providing. And I think you can correctly
> refactor the code as needed - go ahead and make your
> updates and integrate into trunk. I'll recheck it
> after integration.

Ok, I'll do

bkurotsu
Offline
Joined: 2004-12-13

Sorry, I need more time to review this. Working on it...

ersh
Offline
Joined: 2006-10-18