FWD » Core Development » User Interface

Methods and attributes are functions and properties that can be invoked/read/written using a HANDLE type. Each widget or other resource type supports its own lost of methods and attributes.

There is a 4GL "method" called DELETE(). It is available on the following widget types:

• selection list
• combo-box
• radio-set
• browse column

Please write sample 4GL programs that explore how the DELETE() method works. Make sure that the core functionality is well demonstrated by the code.

Then consider the ways one might break DELETE(), or the ways in which it might behave strangely. We need to know its failure conditions, error processing and edge cases.

For an example of how to write 4GL testcases, please see the testcases project, uast/library_calls/. You won't need to write that many tests, but it should show you how to do so. :)

Once you believe you have the entire functionality of DELETE() determined, document those features in this task. We will discuss your results and I suspect we may have some suggestions on further exploration.

Next, you will investigate the related widgets (all 4 of them). The server side is in com/goldencode/p2j/ui/ (e.g. ComboBoxWidget.java) and the client side is in com/goldencode/p2j/ui/client/ (e.g. ComboBox.java). We already have conversion support written for DELETE(), but your job is to implement 4GL compatible runtime functionality.

When you have an implementation plan we can discuss it here and then you can go ahead and write the code.

Post your questions here.

How the COMBO-BOX widget for the BROWSE column can be activated in the Linux character mode?

This is close, but I don't know how to get the cell into editing mode to test it:

def temp-table tt field txt as char.

def query q for tt scrolling.

def browse b query q no-lock
   display tt.txt format "x(10)" view-as combo-box inner-lines 4 list-items "first", "second", "third" 
   enable all
   with size 40 by 10.

def frame f b.

create tt.
tt.txt = "second".
create tt.
tt.txt = "third".

open query q for each tt.

enable all with frame f.
wait-for go of frame f.

FYI, LIST-ITEMS support in P2J may be partial (I think it is missing at least for browse columns).

When running the browse column testcase, how do you get the cell into editing mode?

I failed to activate editing in Linux character mode. In Windows GUI mode the corresponding cell had standard clickable "down" arrow.

Stanislav: in the 4GL, how does one get the ChUI browse cell for a combo-box into editing mode?

You can edit it manually like a normal cell. I'm not sure if drop-down list can actually appear in browse in ChUI. Potentially there can be some event that can be applied to the browse to display drop-down list or scroll between its values, but I couldn't find one.

Good results.

Please make sure you have tested the following:

• For DELETE(list-index): INTEGER inputs of 0 and negative numbers.
• For DELETE(list-index): DECIMAL and INT64 inputs.
• For DELETE(list-items): empty string input.
• For both: unknown value.

Once the results are known for those cases, please review the P2J implementation and start planning the runtime code.

1. The statement about raising error is not correct. Actually the DELETE(list-index) method generates warning if argument == 0 (but not if argument < 0 or > <number of items> - in this case the method just returns FALSE). For RADIO-SET widget the DELETE(label) method also generates warning if the label does not found, is "unknown" or empty.
2. The DELETE(list-items) method just returns FALSE for and empty string (w/o generating a warning). If the list contains non-existing or "unknown" item(s) the method always return FALSE for the COMBO-BOX. For the SELECTION-LIST it return FALSE if the last item in the list is not found or "unknown" and TRUE otherwise.
3. DELETE(list-index) accepts INT64 as argument but not DECIMAL (compilation error)
4. DELETE(list-index) doesn't accept "unknown" value (return FALSE and generates warning)

Runtime support.
1. Server side.
a. Implement delete(int idx), delete(int64 idx), delete(String item) and delete(character item, boolean checkAll) in ControlSetConfig. The additional argument in the last method specifies if TRUE should be returned when all items are found or just a last one.
b. Implement delete(int idx), delete(int64 idx), delete(String item) and delete(character item, boolean checkAll) in ControlSetEntity by delegating to corresponding method of the config field.
c. Implement delete(character item) method in ComboBoxWidget and SelectionListWidget as super(item, true) and super(item, false) respectively.
d. Override delete(int idx), delete(int64 idx) and delete(character item) in RadioSetWidget as
UnimplementedFeature.unsupported("DELETE() method is undefined");
return new logical();

2. Client side.
Nothing is required.

Questions.
1. I understand that we should not care about thread safety of widgets' methods as they are supposed to be invoked in a single even-processing thread as in most GUI frameworks. Is that correct?
2. It is possible in 4GL to suppress warnings by setting SESSION:SUPPRESS-WARNING = TRUE. Should we implement this in Java runtime?

method generates warning if

This means we need some extra investigation. Some warnings are just formatted messages displayed in the message area of the screen but they are not recorded in the error list that can be queried. Other warnings actually get both displayed and recorded and can be queried using the ERROR-STATUS system handle. There is no 4GL documentation that describes any of this. We have just found this behavior through testing. Any such warning behavior should be implemented in the com.goldencode.p2j.util.ErrorManager.java.

In most cases we call ErrorManager.recordOrShowError() to cause the error message to be displayed and recorded.

There is a special case in com.goldencode.p2j.persist.FieldReference.getValue() where we enable/disable "warning mode". In that case the use of ErrorManager.recordOrThrowError() is implemented in the called code and it is not reasonable or correct to change that code to make it handle errors differently. If the warning mode wasn't enabled the error would raise an exception causing a control flow change. By controlling the warning mode, we can eliminate the exception raising part of that behavior.

Constantin/Stanislav: can you think of other ways we code these warning cases in the P2J runtime? Or did I mis-state anything above?

accepts INT64 as argument but not DECIMAL (compilation error)

At this time we don't duplicate any compilation error behavior. In the future, if we ever did a runtime conversion (a kind of JIT for converting dynamically), we would have to honor all the compiler's behavior. But for now, we can ignore this.

Implement delete(int idx)

Since INT64 is possible, it is more correct to use long here.

Implement delete(character item) method in ComboBoxWidget and SelectionListWidget as super(item, true) and super(item, false) respectively.

We need to ensure that anywhere there is something like delete(character) that we also have the corresponding delete(String) implementation to handle classes where string literals are used in the original 4GL. This is the same requirement as how we implement long variants with int64 cases.

d. Override delete(int idx), delete(int64 idx) and delete(character item) in RadioSetWidget as
UnimplementedFeature.unsupported("DELETE method is undefined");
return new logical();

I don't understand why this would be done.

2. Client side.
Nothing is required.

I suspect that the server-side code will need to push the changed state over to the client as part of the delete() method implementation. If the change is immediately visible in the 4GL, then this will be needed otherwise the P2J client won't display it.

I understand that we should not care about thread safety of widgets' methods as they are supposed to be invoked in a single even-processing thread as in most GUI frameworks. Is that correct?

Close. The 4GL is just inherently single threaded. In all things. It has no event thread or other threads at all.

It is true that we don't synchronize these classes because they are only ever accessed from a single thread.

It is possible in 4GL to suppress warnings by setting SESSION:SUPPRESS-WARNING = TRUE. Should we implement this in Java runtime?

Not at this time. When we implement that feature, we will add it to the ErrorManager and globally get the behavior change.

This means we need some extra investigation. Some warnings are just formatted messages displayed in the message area of the screen but they are not recorded in the error list that can be queried. Other warnings actually get both displayed and recorded and can be queried using the ERROR-STATUS system handle. There is no 4GL documentation that describes any of this. We have just found this behavior through testing. Any such warning behavior should be implemented in the com.goldencode.p2j.util.ErrorManager.java.

OK. I will double check.

accepts INT64 as argument but not DECIMAL (compilation error)

At this time we don't duplicate any compilation error behavior. In the future, if we ever did a runtime conversion (a kind of JIT for converting dynamically), we would have to honor all the compiler's behavior. But for now, we can ignore this.

Does it just mean that there is no need to add delete(decimal idx) method?

Implement delete(int idx)

Since INT64 is possible, it is more correct to use long here.

But I see entry(int) and entry(int64) in ControlSetEntity, but there is no entry(long)

Implement delete(character item) method in ComboBoxWidget and SelectionListWidget as super(item, true) and super(item, false) respectively.

We need to ensure that anywhere there is something like delete(character) that we also have the corresponding delete(String) implementation to handle classes where string literals are used in the original 4GL. This is the same requirement as how we implement long variants with int64 cases.

So we'll add delete(String, boolean) and delegate delete(character, boolean) to it like it is done in addFirst().

d. Override delete(int idx), delete(int64 idx) and delete(character item) in RadioSetWidget as
UnimplementedFeature.unsupported("DELETE method is undefined");
return new logical();

I don't understand why this would be done.

Well, RADIO-SET doesn't have delete(idx). The RadioSetWidget has this method by inheritance, so I think it is worth at least to report an error. However the delete(String) and delete(character) methods in RadioSetWidget should be implemented differently for this widget - we have to suppress the conversion of the argument to a list.

2. Client side.
Nothing is required.

I suspect that the server-side code will need to push the changed state over to the client as part of the delete() method implementation. If the change is immediately visible in the 4GL, then this will be needed otherwise the P2J client won't display it.

I see, but this is at server-side anyway.

Does it just mean that there is no need to add delete(decimal idx) method?

Yes. Although how we would handle a signature that needed integer, int64 and decimal is we would just use NumberType which is a superclass to all 3.

But I see entry(int) and entry(int64) in ControlSetEntity, but there is no entry(long)

In practical terms, we may often do that. It is technically incorrect because it is possible to do this:

if not my-widget-handle:delete(999999999999999999999999) then
do:
   /* code that would always execute */
end.

It seems stupid, but you might be surprised at the silliness that we see all the time in real customer code.

So we'll add delete(String, boolean) and delegate delete(character, boolean) to it like it is done in addFirst().

Exactly.

Well, RADIO-SET doesn't have delete(idx). The RadioSetWidget has this method by inheritance, so I think it is worth at least to report an error. However the delete(String) and delete(character) methods in RadioSetWidget should be implemented differently for this widget - we have to suppress the conversion of the argument to a list.

Got it.

I see, but this is at server-side anyway.

Yes, that is right.

Greg Shah wrote:

Constantin/Stanislav: can you think of other ways we code these warning cases in the P2J runtime? Or did I mis-state anything above?

1. use the NO-ERROR clause and check the ERROR-STATUS:ERROR/NUM-MESSAGES-GET-MESSAGE for what is recording
2. to check if ERROR condition is raised, enclose the call in a block like this:
   

   def var l as log init true. /* assume error condition is raised */
   DO ... ON ERROR UNDO, LEAVE:
      h:delete(some-args). /* adjust the arguments so that a error/warning message is shwon */
      l = false.
   END.
   if l <> expected-value-for-this-case then message "error was/was not raised by previous code, when it should have been".
   

In some cases when an ERROR condition is not raised but the message is shown, the recordOrShowError API is use; if NO-ERROR is used, this will also populate the ERROR-STATUS's
attributes. In cases when no ERROR condition is raised and ERROR-STATUS has no recordings about the shown message (when NO-ERROR is used), then displayError is more appropriate.

On a side note, how do you use the r-radio.p test? I've tried switching focus to the frame which reads the to-be-deleted items and I can't do this on ChUI under linux. Do you test on windev01 and use mouse click to focus the frame?

Constantin Asofiei wrote:

On a side note, how do you use the r-radio.p test? I've tried switching focus to the frame which reads the to-be-deleted items and I can't do this on ChUI under linux. Do you test on windev01 and use mouse click to focus the frame?

I've tested both on windev01 and lindev01. In the later case I use ESC TAB to switch between frames

Constantin Asofiei wrote:

To determine what is the message shown by the DELETE method (in this case), usually I check these:

1. use the NO-ERROR clause and check the ERROR-STATUS:ERROR/NUM-MESSAGES-GET-MESSAGE for what is recording
2. to check if ERROR condition is raised, enclose the call in a block like this:
   [...]

In some cases when an ERROR condition is not raised but the message is shown, the recordOrShowError API is use; if NO-ERROR is used, this will also populate the ERROR-STATUS's
attributes. In cases when no ERROR condition is raised and ERROR-STATUS has no recordings about the shown message (when NO-ERROR is used), then displayError is more appropriate.

Well, I've tried both,
With NO-ERROR clause ERROR-STATUS:ERROR is not set.
With DO .. ON-ERROR block the errors is not set but the message is shown.

I understand it means that the warning should be reported via recordOrShowError. Is that correct?

Greg Shah wrote:

But I see entry(int) and entry(int64) in ControlSetEntity, but there is no entry(long)

In practical terms, we may often do that. It is technically incorrect because it is possible to do this:

[...]

It seems stupid, but you might be surprised at the silliness that we see all the time in real customer code.

I see. However at this moment we have delete(int) method in the ControlSetEntity (and even in GenericWidget). According to the recent Greg's mail regarding widgets' hierarchy (if I understood it correctly) I should add delete(long) method to ControlSetEntity only. Is that correct?

Another question is about the semantics of this method. As only int can be used as index for an array (or List) I think that the long value should checked if it is actually an int. If it is the case delete(int) should be invoked, otherwise we just return FALSE (w/o any warning). Is this acceptable?

I see. However at this moment we have delete(int) method in the ControlSetEntity (and even in GenericWidget). According to the recent Greg's mail regarding widgets' hierarchy (if I understood it correctly) I should add delete(long) method to ControlSetEntity only. Is that correct?

Just change the delete(int) to be delete(long)

Another question is about the semantics of this method. As only int can be used as index for an array (or List) I think that the long value should checked if it is actually an int. If it is the case delete(int) should be invoked, otherwise we just return FALSE (w/o any warning). Is this acceptable?

I think it is safe enough to check if the long index is greater that or equal to the size of the array/List. If so it is an invalid index. Anything else can be cast to int since practically speaking, runtime systems don't have the capacity to actually create anything with more elements than an int can handle. Java arrays are already limited to an int as you noted. We have many other locations in the project with this same dependency and I believe it is acceptable for the foreseeable future.

Greg Shah wrote:

Just change the delete(int) to be delete(long)

Got it. So, it seems that I can start coding now. Is that correct?

BTW: I still have uncommitted changes (fixed issues reported by FindBugs). Albeit these changes do not affect the classes I will change in the scope of this task (as far as I can see) it will be convenient for me to have them committed before starting new coding. Can I do this?

So, it seems that I can start coding now. Is that correct?\

Yes.

Albeit these changes do not affect the classes I will change in the scope of this task (as far as I can see) it will be convenient for me to have them committed before starting new coding. Can I do this?

I have not yet gotten the time to finish the code review on those changes (and on the other fix locations you have identified). I do have this on my todo list.

Once the review is done, we will still need to pass regression testing before check in.

For now, if these are in your way, please create a separate p2j checkout for this task.

Greg Shah wrote:

I have not yet gotten the time to finish the code review on those changes (and on the other fix locations you have identified). I do have this on my todo list.

Once the review is done, we will still need to pass regression testing before check in.

For now, if these are in your way, please create a separate p2j checkout for this task.

OK, thank you.

I have a question.
In 4GL warning contains a name of the widget - e.g. "The DELETE attribute on the SELECTION-LIST sl has invalid arguments".
If we're going to generate exactly the same message how to figure the widget name?

Each resource type has a TYPE attribute. It comes from the CommonHandle interface and is implemented for widgets as follows:

HandleResource (implements CommonHandle)
|---HandleChain
    |---GenericWidget

Since all server-side widgets extend from GenericWidget, it can be accessed via getResourceType() which returns a character value.

But HandleResource also provides a simple method type() which returns the attribute as a String. So a simple my-widget.type() should do the trick.

I see. Thank you.

I've implemented DELETE() for RADIO-SET, SELECTION-LIST and COMBO-BOX. I understand that now I have to convert my 4GL sample programs and test. Is that correct?

For the BROWSE column I cannot understand how the combo-box editor is implemented. May be I will understand it looking at the result of conversion of r-browse2.p

I understand that now I have to convert my 4GL sample programs and test. Is that correct?

Exactly right.

Greg,
I cannot perform the conversion:
Exception in thread "main" java.lang.ExceptionInInitializerError
Caused by: java.lang.RuntimeException: com.goldencode.p2j.cfg.ConfigurationException: Error loading p2j.cfg.xml configuration
at com.goldencode.p2j.cfg.Configuration.getInstance(Configuration.java:520)
at com.goldencode.p2j.cfg.Configuration.getParameter(Configuration.java:263)
at com.goldencode.p2j.cfg.Configuration.getParameter(Configuration.java:244)
at com.goldencode.p2j.convert.ConversionDriver.<clinit>(ConversionDriver.java:141)
Caused by: com.goldencode.p2j.cfg.ConfigurationException: Error loading p2j.cfg.xml configuration
at com.goldencode.p2j.cfg.Configuration.getInstance(Configuration.java:521)
... 3 more
Caused by: java.lang.IllegalStateException: Parameter type not supported for setter com.goldencode.p2j.schema.SchemaConfig.setMetadata(): com.goldencode.p2j.schema.SchemaConfig$Metadata (custom behavior may be needed in subclass in order to fix this problem)
at com.goldencode.util.Mapper.genSet(Mapper.java:562)
at com.goldencode.util.Mapper.setProperty(Mapper.java:340)
at com.goldencode.util.Mapper.applyAttributes(Mapper.java:373)
at com.goldencode.util.GenericSAXHandler$ActiveNode.runApplyAttributes(GenericSAXHandler.java:536)
at com.goldencode.util.GenericSAXHandler$ActiveNode.access$0(GenericSAXHandler.java:532)
at com.goldencode.util.GenericSAXHandler.startElement(GenericSAXHandler.java:201)
at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.startElement(AbstractSAXParser.java:509)
at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanStartElement(XMLDocumentFragmentScannerImpl.java:1363)
at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:2786)
at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:606)
at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:510)
at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:848)
at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:777)
at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141)
at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213)
at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:648)
at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl.parse(SAXParserImpl.java:332)
at com.goldencode.p2j.cfg.Configuration.loadXml(Configuration.java:804)
at com.goldencode.p2j.cfg.Configuration.getInstance(Configuration.java:508)
... 3 more

I had to rename p2j.cf.xml.linux to p2j.cfg.xml as p2j.cfg.xml I got after checkout contained references to non-existing files.

When running conversion with a debugger I found that the argument provided as argument for setMetadata() was String "standard"
What I'm doing wrong?
Thank you.

I believe that the current testcases/uast/ environment was broken at some point by some kind of metadata nonsense changes. I think I fixed it by making this change to cfg/p2j.cfg.xml:

-         importFile="data/standard_102b.df" 
-         xmlFile="data/namespace/standard_102b.dict" />
+         importFile="data/standard.df" 
+         xmlFile="data/namespace/standard.dict" />

Thank you very much.
Now the conversion started. However there are other problems at the later steps. I'll try to understand what's wrong.

Well, the conversion passed for combo, radio and select. I had to use f2+m0+cb as f0+cb disn't work (at some stage it complains that cannot load dictionary).
However conversion of browse2.p failed:
EXPRESSION EXECUTION ERROR:
---------------------------
throwException(sprintf(spec, bufname, reftype), this)
^ { Cannot find buffer named p2j_test.book_p2j_test.book for ref type 21! [FIELD_INT id <197568496054> 0:0] }
---------------------------
Elapsed job time: 00:00:00.561
ERROR:
java.lang.RuntimeException: ERROR! Active Rule:
-----------------------
RULE REPORT
-----------------------
Rule Type : WALK
Source AST: [ book-id ] BLOCK/STATEMENT/DEFINE_FRAME/FORM_ITEM/FIELD_INT/ @0:0 {197568496054}
Copy AST : [ book-id ] BLOCK/STATEMENT/DEFINE_FRAME/FORM_ITEM/FIELD_INT/ @0:0 {197568496054}
Condition : throwException(sprintf(spec, bufname, reftype), this)
Loop : false
--- END RULE REPORT ---

at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1004)
    at com.goldencode.p2j.convert.ConversionDriver.processTrees(ConversionDriver.java:953)
    at com.goldencode.p2j.convert.ConversionDriver.back(ConversionDriver.java:814)
    at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1779)
Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:1 [FIELD_INT id=197568496054]
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:225)
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:160)
    at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1500)
    at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1398)
    at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1346)
    at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:972)
    ... 3 more
Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:1
    at com.goldencode.expr.Expression.execute(Expression.java:434)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:401)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.RuleContainer.apply(RuleContainer.java:530)
    at com.goldencode.p2j.pattern.RuleSet.apply(RuleSet.java:50)
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:212)
    ... 8 more
Caused by: com.goldencode.p2j.pattern.CommonAstSupport$UserGeneratedException: Cannot find buffer named p2j_test.book_p2j_test.book for ref type 21! [FIELD_INT id <197568496054> 0:0]
    at com.goldencode.p2j.pattern.CommonAstSupport$Library.throwException(CommonAstSupport.java:2181)
    at com.goldencode.expr.CE4088.execute(Unknown Source)
    at com.goldencode.expr.Expression.execute(Expression.java:341)
    ... 24 more

I guess that P2J is having a problem with DEFINE FRAME f b. from line 10 of r-browse2.p, if that is the same program you are trying to convert.

You can open the AST file and search for the node ID 197568496054, which will bring you to the node being walked at the time of the failure.

I'm not sure if this is a bug or not. You probably ran it in Progress without having the p2j_test database connected. When you are running in P2J, if you have the p2j_test database defined in your p2j.cfg.xml, then it is the equivalent to having that database connected in the 4GL. The "b" reference in the define frame is being matched with a field in the book table of p2j_test, because there must be a field in that table that can be unambiguously matched by the letter "b" abbreviation. This is a "feature" of the 4GL.

You really don't need the p2j_test database to be part of your test environment, so I would just comment it out of your p2j.cfg.xml and try to re-convert.

Greg Shah wrote:

The "b" reference in the define frame is being matched with a field in the book table of p2j_test, because there must be a field in that table that can be unambiguously matched by the letter "b" abbreviation. This is a "feature" of the 4GL.

Yes, I believe this is exactly the problem. It is matching the "b" with the p2j_test.book.book-id field, due to the absurd level of ambiguity which Progress allows (unless we've just implemented it wrong). If you change the "b" to something less ambiguous (even something nearly as short, like "br"), it will convert.

Indeed - after commenting out p2j_test namespace the browse2.p converted OK (even with f0+cb)
Thanks a lot!

If I understand correctly this means that in the scope of this task there is nothing to do specifically for the BROWSE column as the corresponding functionality is just delegated to ComboBoxWidget.

Please upload the Browse2F frame definition class (see the com/goldencode/testcases/ui/ directory).

If I understand correctly this means that in the scope of this task there is nothing to do specifically for the BROWSE column as the corresponding functionality is just delegated to ComboBoxWidget.

For this task I think you can ignore the browse column aspects.

Greg Shah wrote:

For this task I think you can ignore the browse column aspects.

OK. Thank you.

I've tried to launch the P2J Server as described in the "P2J Conversion Handbook", but got an error "Too many configuration files". I started debugging and it seems that there is a bug in the in CommonDriver.processArguments() method - the last line should be return skip+i; instead of return skip. At least after this fix the server started OK.

Is my fix correct or there is another way to deal with the issue?

The command line was
java -XX:MaxPermSize=128m -Xmx768m -Djava.system.class.loader=com.goldencode.p2j.classloader.MultiClassLoader -server -classpath /home/ias/p2j/build/lib/p2jadmin.jar:/home/ias/p2j/build/lib/p2j.jar::. com.goldencode.p2j.main.ServerDriver server.xml net:server:insecure_port=3333 "-z/home/ias/p2j/testcases/uast/build/lib/testcases.jar"

I don't fully understand what you are saying you did or what the problem is.

When I run a server for my converted testcases, here is what I do:

cd /home/ges/projects/p2j_stuff/testcases/simple/server
P2J_HOME=/home/ges/projects/p2j/ ./server.sh -a ../../uast/build/lib/testcases.jar

To see the command that this generates, you can do the following:

cd /home/ges/projects/p2j_stuff/testcases/simple/server
P2J_HOME=/home/ges/projects/p2j/ ./server.sh -t -a ../../uast/build/lib/testcases.jar
cat stdout.log

Which shows that my command line looks like this:

java -XX:MaxPermSize=128m -Xmx768m -Djava.system.class.loader=com.goldencode.p2j.classloader.MultiClassLoader -server -classpath /home/ges/projects/p2j//build/lib/p2jadmin.jar:/home/ges/projects/p2j//build/lib/p2j.jar::../../uast/build/lib/testcases.jar com.goldencode.p2j.main.ServerDriver server.xml net:server:insecure_port=3333

Your command line definitely is wrong, but I'm not sure where or how you even got your command line.

I got the command from /books/conversion_handbook/p2j_conversion_handbook.pdf page 221

The documentation is out of date. The ServerDriver itself has no understanding of a -z parameter. The server.sh has been changed since the time that text was written. Hopefully, with my example above you can properly start the server without your CommonDriver code change.

Yes, it works, thank you.

Good. Sorry for the old docs. We are busy and haven't gotten them updated. And those docs reference an internal bzr project anyway, which changes frequently so it is probably wrong on multiple levels.

You're doing well, keep up the good work.

It's OK - I understand. Thank you,

I've started testing in ChUI mode. Basically all works, but there is one minor(?) issues:

The type() return not the name of the widget, but its type (e.g. "COMBO-BOX") while in 4GL a widget name is shown in the message (e.g. "sl"). As far as I can see in the generated code it not possible to figure the widget name from the widget code as this name is used only as a key in maps in the enclosing WidgetList.

There is a name field in the WidgetConfig, but in my case it is null.

It's not the best way, but for now you can use this: frame.getName(this) from within a widget. This corresponds to the core functionality in GenericWidget.name().

Ultimately, we really should have the original legacy name stored in the config instance.

Thank you - it works. I've finished testing in ChUI mode. Shall I test on Windows as well?

That should not be needed.

OK, so you should do the following:

1. Upload the update zip file here so that we can do a code review.
2. Based on that code review, if there are changes requested you'll make them, re-test if needed and go back to step 1.
3. When it passes code review, you will go into regression testing. See timco.html for details.
4. If there are regressions, make the fixes and go back to step 1.
5. When it passes testing, you will report this in redmine. At this point you will be given the go ahead to check in, but you may also be told to wait until someone else has gone first (especially if there are conflicting changes). In the worst case, this might require a merge and back to step 1. We try to avoid that whenever possible.

Code Review ias_upd20150123.zip

Functionally, it is quite good. Most of the feedback relates to coding standards.

1. BrowseColumnWidget, CommonWidget and GenericWidget are missing history entries.

2. Hard tab characters need to be removed. See ComboBoxWidget, SelectionListWidget, ControlSetEntity and RadioSetWidget. Please configure your editor to replace tabs with spaces (3 spaces per tab). There are also some places where your indents don't follow our standards (each level is 3 spaces).

3. Per our standards, we must not add new 3rd party dependencies to this project without prior approval. For example: com.google.common.collect.Lists; in ControlSetEntity and RadioSetWidget. This is really, really important. The 3rd party dependencies we currently have are scrutinized and reviewed prior to any use. We have both technical and legal requirements to meet before new 3rd party tech can be used.

4. Code like this:

if(idx == 0) {

should be like this:

if (idx == 0)
{

And this:

if( ip == itemList.size() - 1) {

should be this:

if (ip == itemList.size() - 1)
{

And this:

for(int i = 0; i < items.length; i++) {

should be this:

for(int i = 0; i < items.length; i++)
{

And this:

*/  private void invalidDeleteArg() {

should be this:

*/
private void invalidDeleteArg()
{

The above relates to spacing around keywords and always placing the opening methods and blocks (curly braces) on the next line.

5. Copyright dates need to be updated for all updated files that don't already have 2015 listed.

6. Our maximum line length is 98 characters (the coding standards state it is 78 right now, but that is out of date). You have some lines that extend past that limit.

Got it.
Regarding 3rd party dependencies. I used Google Guava classes only because the corresponding jar is already in the build path. Shall I change the code to get rid of using it?

Igor Skornyakov wrote:

Got it.
Regarding 3rd party dependencies. I used Google Guava classes only because the corresponding jar is already in the build path. Shall I change the code to get rid of using it?

Guava is currently in use only by the callgraph generation/TitanDB module. I don't think we should extend its usage outside that so yes, please remove your dependencies.

Sadly, I see that it has also sneaked its way into LogicalTerminal and ChuiWebPageHandler. We'll need to clean those up too at some point.

FYI, we prefer to minimize the linkages because there can be unintended consequences. For example, some of our code runs in our application server, in our application client and in an in-process JVM that runs inside PostgreSQL or SQLServer (this last one via iKVM bytecode conversion to CLR). The UI code you are in right now is only on the server, but I want to avoid extra dependencies.

Code Review ias_upd20150123b.zip

Actually, this update is very hard to compare with the first for ComboBoxWidget, SelectionListWidget, ControlSetEntity and RadioSetWidget. It looks like your editor converted spaces to hard tabs in these files (for 3 of them, in almost the entire file), instead of replacing tabs with spaces. In other words, in these files the changes are the opposite of what was requested. :)

Code Review ias_upd20150123c.zip

I'm sorry.

No worries. Learning the coding standards is part of the ramp-up process.

This is a little better but the Eclipse editor still made many unwanted changes to these files. Use bzr diff on ComboBoxWidget to see what I mean.

I see tow issues.
1. Invalid braces in constructor (fixed)
2. Reformatted comments/Javadocs.
I can revert my changes and then re-apply them (properly formatted).
Is it OK?

The class definition itself has been changed:

< public class ComboBoxWidget
< extends ControlSetEntity<ComboBoxConfig>
< implements AutoZapElement,
<            MaxCharsElement,
<            NumItems,
<            SortedElement,
<            InnerLines
---
> public class ComboBoxWidget extends ControlSetEntity<ComboBoxConfig> implements AutoZapElement,
>       MaxCharsElement, NumItems, SortedElement, InnerLines

I can revert my changes and then re-apply them (properly formatted).
Is it OK?

Certainly. I recommend using meld or diff to carefully check your updates before submitting them. Your editor may "help" you in other ways. ;>

I've tried to run regression testing but the compilation failed. Please find the build logs attached. As I can see the number of compiled Java files (1303) is different from the one in my "normal" build (1497). What I was doing wrong? Please advise. Thank you.

Something is wrong with your repo, see this like:

Tree is up to date at revision 10503 of branch /opt/secure/code/p2j_repo/p2j/trunk

You are on the wrong revision...

I'm very sorry, but I've dropped the entire content of the repo by a stupid mistake. How this can be repaired?

Code Review ias_upd20150124d.zip

Almost there. 5 small problems are remaining. Sorry that we are so specific in our requirements, but we try to achieve very consistent results from the entire team, so that the code and supporting systems/docs look like they were created by a single organism.

1. When you create an update file, the suffix letter should always be there. So instead of ias_upd20150123.zip it would be ias_upd20150123a.zip or ias_upd20150123b.zip etc.

2. The suffix letter in an update zip name is an index into the current day only. It is NOT an index in the updates created for a given task. This means that the first update for a task might in fact be the 3rd update zip you created on a given day, so the update would have a c suffix. In the case of this task, your ias_upd20150124d.zip should have been ias_upd20150124a.zip since this was the first update file you created on the 24th of January.

3. The history entries in BrowseColumnWidget (and CommonWidget) should look like this:

** 022 IAS 20150123          delete(int) replaced with delete(long).
*/

instead of this:

** 022 IAS 20150123          delete(int) replaced with delete(long).*/

4. Import statements should use wildcards unless there is an explicit conflict to resolve. In RadioSetWidget:

import java.util.ArrayList;
import java.util.List;

should be:

import java.util.*;

5. At the end of a class, don't insert an extra blank line between the end of the last method and the end of the class. In ComboBoxWidget line 337, this:

   }

}

should be this:

   }
}

Got it. Thank you for your patience. I've configured Eclipse to avoid specific explicit imports. BTW: what about static imports (so far I didn't use it, just for a future)?
Another question:
I tried to run regression test, but it failed (runtime-regression).
[exec] Non-testing errors (CONNECT_FAILURE) occured during test execution:
[exec] com.jcraft.jsch.JSchException: Auth fail
[exec] at com.jcraft.jsch.Session.connect(Session.java:502)
[exec] at com.goldencode.harness.transport.SSH2Transport.connect(SSH2Transport.java:70)
[exec] at com.goldencode.harness.transport.TransportManager.connect(TransportManager.java:69)
[exec] at com.goldencode.harness.Driver.<init>(Driver.java:83)
[exec] at com.goldencode.harness.TestSet.run(TestSet.java:257)
[exec] at java.lang.Thread.run(Thread.java:744)
[exec]
[exec] Running test plan... * CTRL-C part has failed!
[exec] * Running main part...
[exec]
[exec] Non-testing errors (CONNECT_FAILURE) occured during test execution:
[exec] com.jcraft.jsch.JSchException: Auth fail
[exec] at com.jcraft.jsch.Session.connect(Session.java:502)
[exec] at com.goldencode.harness.transport.SSH2Transport.connect(SSH2Transport.java:70)
[exec] at com.goldencode.harness.transport.TransportManager.connect(TransportManager.java:69)
[exec] at com.goldencode.harness.Driver.<init>(Driver.java:83)
[exec] at com.goldencode.harness.TestSet.run(TestSet.java:257)
[exec] at java.lang.Thread.run(Thread.java:744)
[exec]
[exec] Running test plan... ** MAIN part has failed!

Was I wrong somewhere?

Igor Skornyakov wrote:

I tried to run regression test, but it failed (runtime-regression).

The problem is with your password. Make sure that on this step after run_regression.sh is ran for runtime testing:

system-password:
    [input] Enter system password:

Thank you.

Some regression tests failed:
1. 1. failure in step 13: 'timeout before the specific screen buffer became available (Mismatched data at line 2, column 12. Expected '─' (0x2500 at relative Y 0, relative X 12) and found '' (0x0000 at rel
ative Y 2, relative X 12).)'
2. 1. failure in step 13: 'timeout before the specific screen buffer became available (Mismatched data at line 0, column 1. Expected 'M' (0x004D at relative Y 0, relative X 1) and found '' (0x0000 at relat
ive Y 0, relative X 1).)'
2. dependency chain: tc_item_master_092
3. dependency chain: tc_item_master_092 --> tc_item_master_093
4. failure in step 22: 'timeout before the specific screen buffer became available (Mismatched data at line 3, column 22. Expected '1' (0x0031 at relative Y 3, relative X 22) and found '' (0x0000 at relati
ve Y 3, relative X 22).)'
5. failure in step 40: 'Unexpected EOF in actual at page # 1.'
6. failure in step 8: 'timeout before the specific screen buffer became available (Mismatched data at line 4, column 9. Expected '' (0x0000 at relative Y 4, relative X 9) and found '1' (0x0031 at relative
Y 4, relative X 9).)'
7. failure in step 23: 'timeout before the specific screen buffer became available (Mismatched data at line 4, column 8. Expected '┌' (0x250C at relative Y 4, relative X 8) and found '' (0x0000 at relative
Y 4, relative X 8).)'failure in step 13: 'timeout before the specific screen buffer became available (Mismatched data at line 2, column 12. Expected '─' (0x2500 at relative Y 0, relative X 12) and found '
' (0x0000 at relative Y 2, relative X 12).)'
2. 1. failure in step 13: 'timeout before the specific screen buffer became available (Mismatched data at line 0, column 1. Expected 'M' (0x004D at relative Y 0, relative X 1) and found '' (0x0000 at relat
ive Y 0, relative X 1).)'
2. dependency chain: tc_item_master_092
3. dependency chain: tc_item_master_092 --> tc_item_master_093
4. failure in step 22: 'timeout before the specific screen buffer became available (Mismatched data at line 3, column 22. Expected '1' (0x0031 at relative Y 3, relative X 22) and found '' (0x0000 at relati
ve Y 3, relative X 22).)'
5. failure in step 40: 'Unexpected EOF in actual at page # 1.'
6. failure in step 8: 'timeout before the specific screen buffer became available (Mismatched data at line 4, column 9. Expected '' (0x0000 at relative Y 4, relative X 9) and found '1' (0x0031 at relative
Y 4, relative X 9).)'
7. failure in step 23: 'timeout before the specific screen buffer became available (Mismatched data at line 4, column 8. Expected '┌' (0x250C at relative Y 4, relative X 8) and found '' (0x0000 at relative
Y 4, relative X 8).)'

As far as I can see the failed tests are not related to my changes. Just in case I've started tests again.
I cannot attach complete results as the size of the archive (6.7 MB) exceeds the limit.

Code Review ias_upd20150127a.zip

The changes look good.

I agree that the testing failures are most likely not due to your code. It is a good idea to run testing again and compare results. If the two runs have different failures (except for the expected failure in tc_job_002), then testing is considered passing.

It appears that my understanding of the DELETE method semantics was incorrect -the DELETE(list-items) for COMBO-BOX and SELECTION-LIST widgets delete items by value (not by label). This makes difference when LIST-ITEM-PAIRS is used, not LIST-ITEMS. I will fix it tomorrow.

Code Review ias_upd20150128a.zip

1. We do not yet support Java 8. We want to do so, but it takes some real planning because it requires our customers to switch their production environments. You have added Java 8 dependencies. In addition, we would not use Guava without prior approval. You would have found this pretty quickly when devsrv01 was not able to compile your code for regression testing.

2. As a side note, when you add new data members of a class, our code stds call for these to be at the top of the class (e.g. getValue and getLabel in ControlSetEntity).

3. Line 377 of ControlSetEntity is greater than 98 chars long.

I don't understand. Isn't Function a Java 8 thing or a Guava thing? Neither should be used in our code. Where in Java 7 does Function exist?

Also, please don't upload modified updates as the same update zip file name. The update in note 80 is your second update of the day, ias_upd20150128b.zip.

Sorry, I wasn't looking carefully enough. I see that you defined your own interface to match the concept of the Function in Java 8 or Guava.

Good.

Code Review ias_upd20150128a.zip (which is really ias_upd20150128b.zip)

I'm OK with the changes. Please get them regression tested.

Greg Shah wrote:

Code Review ias_upd20150128a.zip (which is really ias_upd20150128b.zip)

I'm OK with the changes. Please get them regression tested.

The regression test is running

1 tests failed:

2 gso_tests Specific GSO testcases to regression test. FAILED NONE gso_tests 2314.617                                                                                                                     
failure in step 15: 'timeout before the specific screen buffer became available (Mismatched data at line 2, column 9. Expected '┌' (0x250C at relative Y 2, relative X 9) and found '' (0x0000 at relative Y 2, relative X 9).)'

11 tc_tests Specific GSO server TC certification testcases. FAILED NONE tc_tests 12032.162                                                                                                                
1. failure in step 47: 'timeout before the specific screen buffer became available (Mismatched data at line 0, column 0. Expected ' ' (0x0020 at relative Y 0, relative X 0) and found '0' (0x0030 at relative Y 0, relative X 0).)'                                                                                                                                                                                      
2. failure in step 40: 'Unexpected EOF in actual at page # 1.'

Test restarted

Second run of the regression test failed:

2 gso_tests Specific GSO testcases to regression test. FAILED NONE gso_tests 2528.112                                                                                                                     
1. failure in step 53: 'Timeout while waiting for event semaphore to be posted.'                                                                                                                             
2. failure in step 1: 'Timeout while waiting for event semaphore to be posted.'                                                                                                                              
3. failure in step 45: 'timeout before the specific screen buffer became available (Mismatched data at line 21, column 0. Expected 'O' (0x004F at relative Y 21, relative X 0) and found '' (0x0000 at relati
ve Y 21, relative X 0).)'                                                                                                                                                                                    
4. failure in step 37: 'timeout before the specific screen buffer became available (Mismatched data at line 11, column 38. Expected 'R' (0x0052 at relative Y 11, relative X 38) and found 'F' (0x0046 at rel
ative Y 11, relative X 38).)'

2 gso_tests Specific GSO testcases to regression test. FAILED NONE gso_tests 2528.112                                                                                                                     
1. failure in step 53: 'Timeout while waiting for event semaphore to be posted.'                                                                                                                             
2. failure in step 1: 'Timeout while waiting for event semaphore to be posted.'                                                                                                                              
3. failure in step 45: 'timeout before the specific screen buffer became available (Mismatched data at line 21, column 0. Expected 'O' (0x004F at relative Y 21, relative X 0) and found '' (0x0000 at relati
ve Y 21, relative X 0).)'                                                                                                                                                                                    
4. failure in step 37: 'timeout before the specific screen buffer became available (Mismatched data at line 11, column 38. Expected 'R' (0x0052 at relative Y 11, relative X 38) and found 'F' (0x0046 at rel
ative Y 11, relative X 38).)'

11 tc_tests Specific GSO server TC certification testcases. FAILED NONE tc_tests 12896.366                                                                                                                
1. failure in step 22: 'timeout before the specific screen buffer became available (Mismatched data at line 3, column 22. Expected '1' (0x0031 at relative Y 3, relative X 22) and found '' (0x0000 at relati
ve Y 3, relative X 22).)'                                                                                                                                                                                    
2. failure in step 40: 'Unexpected EOF in actual at page # 1.'                                                                                                                                               
3. failure in step 20: 'timeout before the specific screen buffer became available (Mismatched data at line 5, column 18. Expected '' (0x0000 at relative Y 5, relative X 18) and found '┌' (0x250C at relati
ve Y 5, relative X 18).)'                                                                                                                                                                                    
4. failure in step 45: 'timeout before the specific screen buffer became available (Mismatched data at line 21, column 0. Expected 'R' (0x0052 at relative Y 21, relative X 0) and found '' (0x0000 at relati
ve Y 21, relative X 0).)'                                                                                                                                                                                    
5. dependency chain: tc_item_master_006                                                                                                                                                                      
6. dependency chain: tc_item_master_006 --> tc_item_master_007                                                                                                                                               
7. failure in step 34: 'timeout before the specific screen buffer became available (Mismatched data at line 21, column 20. Expected '4' (0x0034 at relative Y 21, relative X 20) and found '3' (0x0033 at rel
ative Y 21, relative X 20).)'                                                                                                                                                                                
8. failure in step 55: 'timeout before the specific screen buffer became available (Mismatched data at line 21, column 0. Expected 'R' (0x0052 at relative Y 21, relative X 0) and found '' (0x0000 at relati
ve Y 21, relative X 0).)'

As we can see the failures are different from the previous run. I understand that regression can be considered as passed. Is that correct?

Are there any specific tests that failed in both runs (even though the failure was at a different point)? The only way to tell this is to look at the test set summary page to see which test numbers failed.

Actually the is one test which failed in exactly the same way in both runs:

failure in step 40: 'Unexpected EOF in actual at page # 1.' (tc_job_002)

All other failed steps are different

Igor Skornyakov wrote:

Actually the is one test which failed in exactly the same way in both runs:

failure in step 40: 'Unexpected EOF in actual at page # 1.' (tc_job_002)

All other failed steps are different

tc_job_002 is expected to fail with this error, so your results are OK.

You can check in the update and send the associated notification email.

Please make sure you have no other changes included in your check in, except what passed testing.

Passed runtime regression testing and checked in as bzr rev 10727.