FWD » Core Development » Base Language

OpenEdge.Core.Collections.ICollection.totable(KW_OUTPUT TABLE-HANDLE) - not all implementations have the 'toTable' method implemented
OpenEdge.Core.Collections.List
OpenEdge.Core.Collections.ObjectStack
Progress.Data.BindingSource
OpenEdge.Core.System.InvalidValueSpecifiedError
progress.util.typehelper
progress.util.resourcemanager
progress.util.enumhelper
openedge.core.collections.listiterator
OpenEdge.Core.TimeStamp.toabldatetimetzfromiso(KW_INPUT character)

ADE does not have source code for the progress package. These need to be tested and implemented in FWD manually.

The other appear in the ADE project:

./abl/src/corelib/OpenEdge/Core/TimeStamp.cls
./abl/src/corelib/OpenEdge/Core/Collections/List.cls
./abl/src/corelib/OpenEdge/Core/Collections/ListIterator.cls
./abl/src/corelib/OpenEdge/Core/Collections/ICollection.cls
./abl/src/corelib/OpenEdge/Core/Collections/ObjectStack.cls
./abl/src/corelib/OpenEdge/Core/System/InvalidValueSpecifiedError.cls

ADE does not have source code for the progress package.

Yes. See #9488-7.

Just one thing might worth mentioning here, the tests we have for OO is based on 12.2 version, there might be differences when we switch to 12.8. The 'collections' are now implemented in Progress and since they even use generics there (only place that this is possible for now) it's probably just extending corresponding Java collections in Progress package and then convert the ones in OpenEdge since those aren't using temp-tables anymore.

ADE also does not have source code for the Ccs package, in FWD these are the skeleton/oo4gl/Ccs files.

in this task we can track the non-ADE .cls implementation (i.e. what is not covered by #9488)

Yes, that makes sense.

Make sure we take any pending work on these classes which exists in 6410b. We may be throwing away the other changes but I guess we have some improvements already.

Constantin Asofiei wrote:

For the first stage, we need to implement the missing progress or ccs classes - double-check the support for these:

• ./skeleton/oo4gl/Ccs/Common/Support/IInt64Holder.cls
  - This is inside the ADE source at ADE/abl/src/corelib/Ccs/Common/Support/IInt64Holder.cls, so this will get covered by #9488.

I will look into 6410b to see what changes are helpful.

I've generated 'skeleton' OO classes for OE 12.8 - as it looks through OO reflection - you can find them in testcases/oo/skeleton/12.8 in revision #1705. Please not all classes/interfaces in OpenEdge.Core.Collections are generic types, this is only possible for Progress 'internal' code right now and not available for us mortals just yet :)

Unfortunately 6410b does not change the status of the support for the classes in #9436-9.

• Progress.Data.ChildrenAllowEdit
• Progress.Data.ChildrenAllowNew
• Progress.Data.ChildrenAllowRemove
• Progress.Data.ChildrenInputValue
• Progress.Data.InputValue
• Progress.Data.CancelCreateRowEventArgs
• Progress.Data.CreateRowEventArgs
• Progress.Data.SortRequestEventArgs
• System.ComponentModel.ComponentResourceManager

I found all of them, except the last System.ComponentModel.ComponentResourceManager, this is not in skeleton/dotnet/System/ComponentModel/ and in testcases/oo there is nothing regarding dotnet.
Marian, do you know if this skeleton exists of how can I generate it?

I think we need a 'hard stop' if you reach .NET classes/behavior. I assume BindingSource or ResourceManager is the 'source' of the problem?

Please rebase 9436a.

9436a is rebased.

skeleton/oo4gl/Ccs/Common/Support/IInt64Holder.cls and the other 'holders' in Ccs package need to be hand-written.

• Added 2 new converted skeletons: Progress.IO.MemoryOutputStream, Progress.IO.ByteOrder, needed on #9488.

Committed on 9436a rev. 15753 - Added converted skeletons for multiple classes from Ccs.Common.Support, OpenEdge.Core, OpenEdge.Core.System, OpenEdge.Core.Util, OpenEdge.Logging, OpenEdge.Security:

• p2j/oo/common/support/IdateTimeTzHolder.java
• p2j/oo/common/support/IdecimalHolder.java
• p2j/oo/common/support/Iint64Holder.java
• p2j/oo/common/support/IlongcharArrayHolder.java
• p2j/oo/common/support/IprimitiveArrayHolder.java
• p2j/oo/core/DataTypeHelper.java
• p2j/oo/core/EventArgs.java
• p2j/oo/core/ImemoryPointer.java
• p2j/oo/core/LegacyDecimal.java
• p2j/oo/core/LegacyInteger.java
• p2j/oo/core/LongcharArrayHolder.java
• p2j/oo/core/StringConstant.java
• p2j/oo/core/TimeStampConstants.java
• p2j/oo/core/system/InvalidValueSpecifiedError.java
• p2j/oo/core/system/NotFoundError.java
• p2j/oo/core/util/Token.java
• p2j/oo/core/util/TokenResolver.java
• p2j/oo/core/util/TokenResolverEventArgs.java
• p2j/oo/logging/LogEvent.java
• p2j/oo/security/Principal.java

This completes the list from #9488-78, now all classes exist in p2j.
Note: I added some LegacyResourceSupport by hand. If the annotation is also needed at PROPERTY level, I will make the necessary changes to conversion.

9436a is merged to trunk as rev. 15782 and archived.

Below is an updated list of the current status, what tests exist and extra classes that need to be implemented from #9488 dependencies.

To implement¶

To complete¶

To test¶

Just to be clear, classes with .NET dependencies will not be implemented.

This is the latest list of what is needed from the Progress package:

STUB¶

• ./skeleton/oo4gl/Progress/Lang/LockConflict.cls - existent in p2j at com.goldencode.p2j.oo.lang.LockConflict
• ./skeleton/oo4gl/Progress/Reflect/Constructor.cls - existent in p2j at com.goldencode.p2j.oo.reflect.LegacyConstructor
• ./skeleton/oo4gl/Progress/Lang/OERequestInfo.cls - existent in p2j at com.goldencode.p2j.oo.lang.OerequestInfo
• ./skeleton/oo4gl/Progress/Lang/Stop.cls - existent in p2j at com.goldencode.p2j.oo.lang.Stop
• ./skeleton/oo4gl/Progress/Reflect/Property.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Property
• ./skeleton/oo4gl/Progress/Reflect/Variable.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Variable

PARTIAL/BASIC¶

• ./skeleton/oo4gl/Progress/Json/ObjectModel/JsonArray.cls - existent in p2j at com.goldencode.p2j.oo.json.objectmodel.JsonArray - rt PARTIAL.
• ./skeleton/oo4gl/Progress/Lang/Class.cls - existent in p2j at com.goldencode.p2j.oo.lang.LegacyClass - rt PARTIAL.
• ./skeleton/oo4gl/Progress/Lang/ParameterList.cls - existent in p2j at com.goldencode.p2j.oo.lang.ParameterList - rt BASIC.
• ./skeleton/oo4gl/Progress/Lang/ProError.cls - existent in p2j at com.goldencode.p2j.oo.lang.ProError - rt BASIC.
• ./skeleton/oo4gl/Progress/Reflect/Event.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Event - rt PARTIAL.
• ./skeleton/oo4gl/Progress/Reflect/Method.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Legacy4GLMethod - reported STUB but seems implemented.

FULL¶

• ./skeleton/oo4gl/Progress/ApplicationServer/AdapterTypes.cls - existent in p2j at com.goldencode.p2j.oo.applicationserver.AdapterTypes - reported STUB but it is enum
• ./skeleton/oo4gl/Progress/Json/JsonParser.cls - existent in p2j at com.goldencode.p2j.oo.json.JsonParser
• ./skeleton/oo4gl/Progress/Json/ObjectModel/JsonConstruct.cls - existent in p2j at com.goldencode.p2j.oo.json.objectmodel.JsonConstrunct
• ./skeleton/oo4gl/Progress/Json/ObjectModel/JsonObject.cls - existent in p2j at com.goldencode.p2j.oo.json.objectmodel.JsonObject
• ./skeleton/oo4gl/Progress/Json/ObjectModel/ObjectModelParser.cls - existent in p2j at com.goldencode.p2j.oo.json.objectmodel.ObjectModelParser
• ./skeleton/oo4gl/Progress/Lang/AppError.cls - existent in p2j at com.goldencode.p2j.oo.lang.AppError
• ./skeleton/oo4gl/Progress/Lang/Enum.cls - existent in p2j at com.goldencode.p2j.oo.lang.LegacyEnum
• ./skeleton/oo4gl/Progress/Lang/Error.cls - existent in p2j at com.goldencode.p2j.oo.lang.LegacyError
• ./skeleton/oo4gl/Progress/Lang/FlagsEnum.cls - existent in p2j at com.goldencode.p2j.oo.lang.FlagsEnum
• ./skeleton/oo4gl/Progress/Lang/Object.cls - existent in p2j at com.goldencode.p2j.oo.lang.BaseObject
• ./skeleton/oo4gl/Progress/Lang/OEVersionInfo.cls - existent in p2j at com.goldencode.p2j.oo.lang.OeversionInfo
• ./skeleton/oo4gl/Progress/Lang/SysError.cls - existent in p2j at com.goldencode.p2j.oo.lang.ProError
• ./skeleton/oo4gl/Progress/Reflect/AccessMode.cls - existent in p2j at com.goldencode.p2j.oo.reflect.AccessMode
• ./skeleton/oo4gl/Progress/Reflect/Flags.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Flags
• ./skeleton/oo4gl/Progress/Reflect/Parameter.cls - existent in p2j at com.goldencode.p2j.oo.reflect.Parameter
• ./skeleton/oo4gl/Progress/Reflect/ParameterMode.cls - existent in p2j at com.goldencode.p2j.oo.reflect.ParameterMode
• ./skeleton/oo4gl/Progress/Reflect/DataType.cls - existent in p2j at com.goldencode.p2j.oo.reflect.DataType

Thanks, for now work on the ADE dependencies for Progress package.

• rev. 15826: Corrected support level for IprimitiveArrayHolder, IlongcharArrayHolder, Iint64Holder, IdateTimeTzHolder, AdapterTypes.
• rev. 15827: Included constructor with 2 parameters in ArrayIterator added in newer versions of Progress.

Besides the new constructor ArrayIterator is now implementing IListIterator. This is the diff of the skeletons:

=== modified file 'oo4gl/OpenEdge/Core/Collections/ArrayIterator.cls'
--- old/oo4gl/OpenEdge/Core/Collections/ArrayIterator.cls    2020-07-29 11:20:46 +0000
+++ new/oo4gl/OpenEdge/Core/Collections/ArrayIterator.cls    2025-02-17 08:43:14 +0000
@@ -1,9 +1,13 @@
 class OpenEdge.Core.Collections.ArrayIterator
-    implements OpenEdge.Core.Collections.IIterator:
+    implements OpenEdge.Core.Collections.IListIterator, 
+        OpenEdge.Core.Collections.IIterator:

     constructor public ArrayIterator ( poIteratedArray as OpenEdge.Core.Collections.Array):
     end.

+    constructor public ArrayIterator ( pIteratedArray as OpenEdge.Core.Collections.Array, pIndex as integer):
+    end.
+
     define protected property CurrentExtent as integer no-undo
         protected get.
         protected set.
@@ -15,9 +19,21 @@
     method public logical HasNext ():
     end.

+    method public logical HasPrevious ():
+    end.
+
     method public Progress.Lang.Object Next ():
     end.

+    method public integer NextIndex ():
+    end.
+
+    method public Progress.Lang.Object Previous ():
+    end.
+
+    method public integer PreviousIndex ():
+    end.
+
     method public logical Remove ():
     end.

We need to implement these as well.

Progress.Collections.List implements Progress.Collection.IIterable, with method getIterator(), in Progress the type of this iterator is Progress.Collections.ListIterator<Progress.Lang.Object>. We need to implement Progress.Collections.ListIterator as well. This is it's skeleton:

class Progress.Collections.ListIterator
        implements Progress.Collections.IIterator final:

        define public property Current as Progress.Lang.Object no-undo
                public get.

        method public logical MoveNext ():
        end.

        method public void Reset ():
        end.

end.

• having list [a, b, c, d]
• Current = b
• MoveNext finds the location of Current in the list and gets the next value, so it will be c
• but what happens with [a, b, c, b, d]? - please experiment.

Otherwise, we can save the collection instance ListIterator is walking, and just use a Java iterator over the collection.

But how do I get the list in the iterator? I'll write some unit-tests.

https://docs.progress.com/bundle/abl-reference/page/Progress.Collections.ListIterator-ltTgt-class.html

Try to get the constructor via reflection and check its signature.

• Completed implementation for Progress.Collection.List.
• Added Progress.Collection.ListIterator with completed implementation.
• Modified Icomparable to use generics.
• Added in BasaDataType the hashCode function, the implementation for the new hash-code() builtin function - now I use Objects.hashcode. I will test this more in Progress and fully implement it.

Committed to 9436b rev.15854: completed implementation of OpenEdge.Core.Collections.ArrayIterator. This is an example of class that is different between Progress versions. The diff of the stubs is posted in #9436-29.

For implementing, I first went and tested with Progress 12.8 the existent tests at /testcases/tests/oo/openedge/core/collections/array_iterator. These were failing(most of them). I adjusted them to pass and added tests for the new constructor and methods. Converted and worked on the implementation. These tests are dependent on OpenEdge.Core.Collections.Array that is only STUB in fwd. I used the converted class we got in #9488(had some issues here also but I will talk about them in a separate post on #9488).
I adjusted the existent implementation for the next() function, it has an extra Assert before the actual implementation: Assert.isTrue(ArrayIterator.this.hasNext());. and some logic. Besides that I just added the new function and the new constructor.

I tested now only the TestConstructor.TestConstructorArray, and in older version of progress it is correct like this:

oAI = new ArrayIteratorStub(oArray) no-error.
support.test.AssertExt:Error(0, 'Iterated Array cannot be null'). // Constructor with null param should throw error

oAI = new ArrayIteratorStub(oArray) no-error.
support.test.AssertExt:Error(0, 'Iterated Array cannot be unknown'). // Constructor with null param should throw error

Greg/Constantin, please advice.

I am not sure how to handle these:
1. in testcases: Should I modify the existent tests? Not sure if that is ok, they seem to be correct for v11. I was thinking of creating /testcases/oo/openedve/v12/.. and commit my tests there instead of modifying the already existent tests.
2. In fwd I am not sure on how to tread this. I want to check all the tests(not only the TestConstructor) to see that they work in v 11, convert them and test in fwd also to see if my ArrayIterator implementation(that works for v12) is messing things up. I think it will, especially for the next method.

The key question here: is the core test code the same between these versions? Can the differences between versions be handled by small amounts of conditional code?

If so, then our preferred approach is to implement some form of conditional processing based on the OE version. This might be at preprocessing time (if needed) but it would be better if this could be a runtime decision (using the version methods in EnvironmentOps).

The preprocessor solution should only be used for code that cannot compile (in OE) without such conditional code. Otherwise, if the code will compile in all versions of OE then add runtime code to check the version and drop through different logic as needed.

I don't want to duplicate the test code by version unless the test code itself is drastically different.

Ioana-Cristina Prioteasa wrote:

Committed to 9436b rev.15854: completed implementation of OpenEdge.Core.Collections.ArrayIterator. This is an example of class that is different between Progress versions. The diff of the stubs is posted in #9436-29.

For implementing, I first went and tested with Progress 12.8 the existent tests at /testcases/tests/oo/openedge/core/collections/array_iterator. These were failing(most of them).

Those were already fixed for OE12.8, most of the issues are coming from stupid changes on error messages (they seems to have a real problem of what word to use 'null' or 'unknown'). So far we've used different asserts based on the OE version (runtime check), for other more complex changes like methods that were removed/renamed where we had to use preprocessor to make the code compile on both versions.

As Greg said his preference is to not duplicate the tests for each OE version, this might lead to somehow more complicated tests down the path but till the number of differences isn't that high this will probably work.

Anyway, the changes were committed in testcases in rev #1734.

I might be doing something wrong, I am not not running the tests in the testcases project. I convert them separately and run them.
I see the changes in testcases-1734 mostly add files in tests/preproc, does not change the actual tests. For these changes to take effect I need to run them inside testscases project? Do I need to add some special configuration? I am also a bit unsure how to test in Progress. Do I need the whole project?

• the formula used is:
  

  result = 1;
  for each argument arg:
     result = 31 * result + HASH-CODE(arg);
  

  
  But this is for when we have 2 or more arguments, for one argument we should return only the hash, not 31 + hash. I used a getIndividualHash(BaseDataType arg) helper method that uses arg.hashCode(), overridden function from BaseDataType to compute the individual hash.
• for unknown types: HASH-CODE(?) = 0
• A more interesting thing: HASH-CODE (5) = HASH-CODE (5.00), so if a decimal is equivalent to an integer, we need to use the integer hash.

For now my tests focus more on the properties of this and how it is calculated when we have multiple arguments, not exactly on how the individual HASH-CODE is computed.
I experimented briefly in this area and I found that HASH-CODE(1)=1, HASH-CODE(40)=40, and so on for integer types. Our implementation for integer.hashCode() does not follow these rules.
So if I were to check the exact value returned, tests would not pass.
I am not sure about modifying the already implemented BaseDataType.hashCode() for each type.
Any opinions on this? Should I start looking into how each data type's hash looks?

In the meantime, I committed my changes to 9436b, rev.15855, it contains conversion and runtime support for this, and with this revision my unit tests pass.
The conversion support is also in 9488a, rev. 15850, but I had a small conversion bug, when I would pass HASH-CODE (1), it was not wrapped in integer. I fixed that in 9436b.

Committed on testcases project rev. 1744: Added tests for Progress.Collections.List and built-in function HASH-CODE.

• Running in our VM with older progress version:
  • TestConstructor: passes
  • TestMethodHasNext: passes
  • TestMethodNext: fails
    

            // iterate array and check method ----------------------
            do while oAI:HasNext():
                Assert:Equals(counter, oAI:GetCurrentExtent()). // CurrentExtent before Next should be as expected
    
                oPLO = oAI:Next() no-error.
                support.test.AssertExt:NotErrorNotWarning(). // method call inside array size limits should not throw error ------------------------------> it fails here
    
                Assert:IsFalse(valid-object(oPLO)). // because array is empty response should be invalid object
                Assert:Equals(counter + 1, oAI:GetCurrentExtent()). // Next should increase CurrentExtent property by 1
    
                counter = counter + 1.
            end.
    

    
    I eliminated no error and the error I get is: Extent cannot be zero.. I fixed this part like this:
    

                Assert:Equals(counter, oAI:GetCurrentExtent()). // CurrentExtent before Next should be as expected
    
                if oAI:GetCurrentExtent() EQ 0 then do:
                   oPLO = oAI:Next() no-error.
                   support.test.AssertExt:Error(0, 'Extent cannot be zero'). 
                end.
                else do:
                   oPLO = oAI:Next() no-error.
                   support.test.AssertExt:NotErrorNotWarning().
                end.
    

    
    This construct appears 2 times in the test. And both times it gives error on first call when extent is 0.
    Next issue, this confuses me a bit. After the hasNext while, calling next one more time does not give error:
    

            oPLO = oAI:Next() no-error.
            if decimal(proversion) >= 12.8 then
                support.test.AssertExt:Error(0).
            else
                //support.test.AssertExt:Error(11388, 'Array subscript 11 is out of range.  The indeterminate extent is fixed to a dimension of 10. (11388)'). // method call for outside of array size limit should throw error -------------------------> was failing here
                support.test.AssertExt:NotErrorNotWarning(). ------------------------> Now passing
    

  • TestMethodRemove: this does not compile because of this(used 3 times) Assert:Equals(?, lRsp). Equals cannot have unknown parameter. This needs to be replaced with: Assert:IsUnknown(lRsp).
    After correcting this, it fails here:
    

            // check method ----------------------
            lRsp = oAI:Remove() no-error.
            //support.test.AssertExt:Error(0, 'Current iterator position cannot be zero'). ----------------> fail here
            support.test.AssertExt:Error(0, 'Extent cannot be zero'). -------------------------------------> corrected
            Assert:IsUnknown(lRsp). 
    

    
    Then here, when counter = oArray:Size error is thrown:
    

            do counter = 1 to oArray:Size:
                oArray:SetValue(oStr, counter).
            end.
    

    
    To correct it we need: do counter = 1 to oArray:Size - 1:.
    Another fail here:
    

            oAI = new ArrayIteratorStub(oArray) no-error.
            counter = 1.
    
            do while oAI:HasNext():
                oPLO = oAI:Next() no-error.
    
                if valid-object(oPLO) then 
                do:
                    lRsp = oAI:Remove() no-error.
    
                    if counter = 1 then 
                        support.test.AssertExt:NotErrorNotWarning(). // on first call method should not throw error
                    else 
                        if decimal(proversion) >= 12.8 then
                            support.test.AssertExt:NotErrorNotWarning().  
                        else
                            support.test.AssertExt:Error(10068, 'Lead attributes in a chained-attribute expression (a:b:c) must be type HANDLE or a user-defined type and valid (not UNKNOWN). (10068)'). //  method should throw error
                    Assert:IsTrue(lRsp). // if object was removed then response should be true
                end.
                counter = counter + 1.
            end.
    

    
    I fixed this like:
    

            oAI = new ArrayIteratorStub(oArray) no-error.
            counter = 1.
    
            do while oAI:HasNext():
                oPLO = oAI:Next() no-error.
    
                if valid-object(oPLO) then 
                do:
                    lRsp = oAI:Remove() no-error.
    
                    if counter = 1 then 
                        support.test.AssertExt:NotErrorNotWarning(). // on first call method should not throw error
                    else 
                        if decimal(proversion) >= 12.8 then do:
                            support.test.AssertExt:NotErrorNotWarning(). 
                            Assert:IsTrue(lRsp). // if object was removed then response should be true 
                        end.
                        else do:
                            MESSAGE(counter).
                            if counter = 2 then do:
                                support.test.AssertExt:NotErrorNotWarning().
                                Assert:IsTrue(lRsp). // if object was removed then response should be true
                            end.
                            else do:
                                support.test.AssertExt:Error(10068, 'Lead attributes in a chained-attribute expression (a:b:c) must be type HANDLE or a user-defined type and valid (not UNKNOWN). (10068)'). //  method should throw error
                                if counter = oArray:Size + 1 then
                                Assert:IsFalse(lRsp).
                                else
                                Assert:IsTrue(lRsp). // if object was removed then response should be true
                            end.
                        end.
    
                end.
                counter = counter + 1.
            end.
    

    
    The last issue here:
    

            oAI = new ArrayIteratorStub(oArray) no-error.
            counter = 1.
            do while oAI:HasNext():
                oPLO = oAI:Next() no-error.
    
                if counter = 1 then 
                    if decimal(proversion) >= 12.8 then
                         Assert:IsTrue(valid-object(oPLO)).
                    else
                        Assert:IsFalse(valid-object(oPLO)). // first object should be removed
                else        
                    Assert:IsTrue(valid-object(oPLO)). // rest of the objects should not be removed
                counter = counter + 1.
    

    
    Fixed:
    

            oAI = new ArrayIteratorStub(oArray) no-error.
            counter = 1.
            do while oAI:HasNext():
                oPLO = oAI:Next() no-error.
    
                if counter = 2 then 
                    if decimal(proversion) >= 12.8 then
                         Assert:IsTrue(valid-object(oPLO)).
                    else
                        Assert:IsFalse(valid-object(oPLO)). // first object should be removed
                else        
                    Assert:IsTrue(valid-object(oPLO)). // rest of the objects should not be removed
                counter = counter + 1.
            end.
    

  • TestPropertyCurrentExtent: passes
  • TestPropertyIteratedArray: passes

I am thinking of applying these correction and move to testing these in version 12 as well. If more mismatch found use the version check if decimal(proversion) >= 12.8 then do:. I will also retest the new tests for the new methods only in v12 in older version and just skip them if version is smaller.

For integrating the v12 tests I think I need to use the preprocessor. For example in ArrayIteratorStub I need to add this:

    constructor public ArrayIteratorStub ( input poIteratedArray as Array, input pIndex as integer):
        super (input poIteratedArray, input pIndex).
    end constructor.

Ioana-Cristina Prioteasa wrote:

Marian, I am now looking at the tests for OpenEdge.Core.Collections.ArrayIterator at tests/oo/openedge/core/collections/array_iterator/:

• Running in our VM with older progress version:

How old? In any case the baseline used for writing tests (ABLUnit style) was OE12.2, ABLUnit code as well as any OO code provided by Progress has changes between versions and we did not test with all versions not wrote the tests such a way that it will pass in all versions. We had to upgrade to 12.8 our development environment and we've made those tests pass in 12.2 and 12.8 using preprocessor or simple runtime check on proversion. There is no guaranty the tests will pass in any of the versions between 12.2 and 12.8 nor future versions, in fact those should only pass in 12.2 and 12.8. Using the same technique to use the same tests for all (maybe just supported) versions is going to be messy on the long run for sure.

I am thinking of applying these correction and move to testing these in version 12 as well. If more mismatch found use the version check if decimal(proversion) >= 12.8 then do:. I will also retest the new tests for the new methods only in v12 in older version and just skip them if version is smaller.

You can definitively try to update the tests to make it pass in older versions, or any other version needed apart from 12.2 and 12.8. I don't particularly like this approach but it's not my call here, it just makes those tests unnecessary complex and hard to understand.

One thing about using assertions is we already have one AssertExt support class that we use for error checking, if the tests must be able to run in all (supported) OE versions then it should be easier to move all assertion methods in that class and call the appropriate Assert method based on the OE version from there - like an abstraction layer to avoid doing that in the tests (like Equals with unknown, IsUnknown, IsNull, whatever PSC chooses to name those assert methods over time).

Ioana-Cristina Prioteasa wrote:

For integrating the v12 tests I think I need to use the preprocessor. For example in ArrayIteratorStub I need to add this:

This gives compilation error in v11, while in v12 works perfectly. Is there an example in testcases where something like this is done? I am not exactly sure how to go about it.

Yes, please see tests.oo.openedge.core.collections.list.ListStub - the only way to go here is to use preprocessor :(

Marian Edu wrote:

How old? In any case the baseline used for writing tests (ABLUnit style) was OE12.2, ABLUnit code as well as any OO code provided by Progress has changes between versions and we did not test with all versions not wrote the tests such a way that it will pass in all versions. We had to upgrade to 12.8 our development environment and we've made those tests pass in 12.2 and 12.8 using preprocessor or simple runtime check on proversion. There is no guaranty the tests will pass in any of the versions between 12.2 and 12.8 nor future versions, in fact those should only pass in 12.2 and 12.8. Using the same technique to use the same tests for all (maybe just supported) versions is going to be messy on the long run for sure.

I tested with 11.6..
Seems like if we want these tests to work on all supported versions they will get endlessly more complicated. I am not sure how to go about this. If what is on xfer works on 12.2(i am not sure how I can access that version to test), but don't work in v11.6 nor on v 12.8, this complicates things a lot.

Ioana-Cristina Prioteasa wrote:

If what is on xfer works on 12.2(i am not sure how I can access that version to test), but don't work in v11.6 nor on v 12.8, this complicates things a lot.

The tests should pass in 12.8, well we do use 12.8.6 to be precise. If you have a different 12.8 version and find tests that don't pass please let us know.

We have customers that are on 11.x and will never go to 12.x. It is critical that the tests can work across multiple versions. We will NOT worry about anything other than an LTS release on 12.x (so 12.2 and 12.8). Yes, 11.x must be supported.

Greg Shah wrote:

Yes, 11.x must be supported.

I don't really see the benefits, other than testing PSC code on OE, since the implementation in FWD is based on converting version 12.8 but I'm sure I'm missing the bigger picture :)

We have customer applications that can only run in 11.x. Those applications will be compared with FWD using the OE 11.x environments.

9436b was rebased from trunk rev 15875 - new rev 15881.

• MemoryOutputStream
  • please update the support annotations for MemoryOutputStream
  • is there a destructor in MemoryOutputStream, which automatically de-allocates the data memptr? Try a test like this: create an instance, allocate data and write something, get the data/memptr, delete the instance - is the data still in memory?
  • do not use bytesWritten = new int64(0); or such, use .assign methods; the point is: the instances are mutable and the actual (field/var) Java reference is immutable. For example, please fix clear_
  • setByteOrder_1 - the setByteOrder(_value); call is called outside of the internalProcedure code
  • instead of new logical(false) use logical.FALSE
  • public int64 write(final longchar _value, final logical _includeNull), and all others - the code needs to be inside an internalFunction or internalProcedure (if returns void) declaration - this is missing.
• memptr.setLength - are the changes only for the MemoryOutputStream, or is this actually for the set-size(memptr) statement, too? This is for the resizing the memptr.
• should Icomparable<T> be Icomparable<T extends _BaseObject_>? I assume you can't use 4GL native types like character.
• oo.collections.List
  • use logical.TRUE and such, instead of i.e. new logical(true
  • addAll and others - use internalFunction or internalProcedure (if returns void)
• oo.collections.ListIterator
  • moveNext and others - use internalFunction or internalProcedure (if returns void)
• core.collections.ArrayIterator
  • why use Assert functions? Are these actual exceptions thrown by the runtime?
• the i.e. __collections_listIterator_constructor__ also need to use internalProcedure and also need to call explicitly __lang_BaseObject_constructor__ - check all changed files in this task!
• all methods which are from 4GL need to have internalFunction or internalProcedure - please double-check the files changed in this task.

In List.addAll I had:

public logical addAll(final object<? extends Icollection> otherCollections)
{
   boolean addedAny = false;

   if (otherCollections.isUnknown())
   {
      ErrorManager.recordOrThrowError(18193,
                                      "Invalid value specified for parameter 'otherCollections' of method or constructor 'addAll'",
                                      false);
      return new logical(false);
   }

public logical addAll(final object<? extends Icollection> otherCollections)
{
   return function(List.class, this, "AddAll", logical.class, new Block((Body) () ->
   {
      boolean addedAny = false;

      if (otherCollections.isUnknown())
      {
         ErrorManager.recordOrThrowError(18193,
                                         "Invalid value specified for parameter 'otherCollections' of method or constructor 'addAll'",
                                         false);
         returnNormal(logical.FALSE);
      }

silent(() -> result.assign(targetList.ref().addAll(new object())));
         com.goldencode.dataset.support.test.AssertExt.error(new integer(18193), new character("Invalid value specified for parameter 'otherCollections' of method or constructor 'AddAll'. (18193)"));

How is the test executed? With NO-ERRO?

Yes, and it gets converted as:

silent(() -> result.assign(targetList.ref().addAll(new object())));

com.goldencode.dataset.support.test.AssertExt.error(new integer(18193), new character("Invalid value specified for parameter 'otherCollections' of method or constructor 'AddAll'. (18193)"));

Had a meeting with Constantin about this and it seems that the methods in the Progress package are not wrapped in a body. I removed all the internalProcedure and internalFunction from the classes I modified. I retested the tests I had and everything works as expected.

• is there a destructor in MemoryOutputStream, which automatically de-allocates the data memptr?
• memptr.setLength - are the changes only for the MemoryOutputStream, or is this actually for the set-size(memptr) statement, too? This is for the resizing the memptr. -- I need to test this properly for memptr as well. I used it directly in MemoryOutputStream like data.setLength(required); ant it did not behave well in my case. I will check set-size.

But I committed the partially addressed review in 9436b rev. 15882. I will move to commit to 9436b what in needed from 9488. I will revisit then the above points.

• rev. 15883: Support *=, -=, +=, /= operator syntax.
• rev. 15884: Support for final modifier for class property.
• rev. 15885: Conversion support and STUB runtime support for ClientPricipal:TENANT-ID and ClientPricipal:TENANT-NAME.
• rev. 15886: Support for package-private access modifier.
• rev. 15884: Bug fixes: resize needs an integer argument for the index, read extent from an EXPRESSION node, fixed the names for the extent property Java methods when loading a builtin .cls from FWD p2j.oo.

Hopefully I understood correctly and you wanted the last 3 in the same commit.. but now that I think about it.. Should I uncommit the last one or revert it and do 3 different commits?

&if integer(entry(1, proversion, '.')) ge 12 &then
        assign extent(this-object:Value) = pNewLength.
&else
        assign extent(oTempObject)       = this-object:Length
               oTempObject               = this-object:Value
               iMax                      = min(this-object:Size, pNewLength)
               extent(this-object:Value) = ?
               extent(this-object:Value) = pNewLength
               .
        /* On init this loop won't execute */
        do iLoop = 1 to iMax:
            assign this-object:Value[iLoop] = oTempObject[iLoop].
        end.
&endif

if (_isGreaterThanOrEqual(new decimal(EnvironmentOps.getVersion()), decimal.fromLiteral("12.0")))

oArray = new Array() no-error.

if decimal(proversion) >= 12.8 then
   Assert:Equals(0, oArray:Size).
else
   Assert:Equals(10, oArray:Size).

   public void __core_collections___core_collections_Array_constructor__static___constructor__()
   {
      //  we don't have any elements yet
      internalProcedure(Array.class, this, "Array", new Block((Body) () ->
      {
         __core_collections_Array_constructor__(Array.getDefautArraySize());
         if (_isGreaterThanOrEqual(new decimal(EnvironmentOps.getVersion()), decimal.fromLiteral("12.0")))
         {
            mSize.assign(0);
         }
      }));
   }

• Extent cannot be zero.
  Progress test:
  

  // add objects in array ---------------------------------
          oStr = new String('string').
  
          oArray:SetValue(oStr, 3).
          oArray:SetValue(oStr, 5).
          oArray:SetValue(oStr, 8).
  
          oAI = new ArrayIteratorStub(oArray) .
          counter = 0.
  
          do while oAI:HasNext():
              Assert:Equals(counter, oAI:GetCurrentExtent()). // CurrentExtent before Next should as expected
  
             if oAI:GetCurrentExtent() EQ 0 AND decimal(proversion) < 12.8 then do:
                 oPLO = oAI:Next() .
                 support.test.AssertExt:Error(0, 'Extent cannot be zero'). 
              end.
              else do:
                 oPLO = oAI:Next() no-error.
                 support.test.AssertExt:NotErrorNotWarning().
              end.
  

  
  In Progress the Stack trace shows:
  - ArrayIterator.next
  - Array.getValue
  - Assert.NotNullorZero
  - Assert.NotZero.
  But it makes no sense, this is ArrayIterator.next
  

     @LegacySignature(returns = "OBJECT", type = Type.METHOD, name = "Next", qualified = "progress.lang.object")
     @LegacyResourceSupport(supportLvl = CVT_LVL_FULL | RT_LVL_FULL)
     public object<? extends com.goldencode.p2j.oo.lang._BaseObject_> next_()
     {
        return function(ArrayIterator.class, this, "Next", object.class, new Block((Body) () ->
        {
           Assert.isTrue(ArrayIterator.this.hasNext());
           setCurrentExtent(InputParameter.property(integer.class, plus(getCurrentExtent(), 1)));
           storeReturnValue(((Array) getIteratedArray().ref()).getValue(getCurrentExtent()));
        }));
     }
  

  
  So I have the extent 0 but it sets it to 1 before calling the getValue..
• In the remove method I should get a Lead attributes in a chained-attribute expression (a:b:c) must be type HANDLE or a user-defined type and valid. I am not exactly sure how.. In the Progress stackTrace I see it's coming from Array.Remove.
  This is the method now:
  

     @LegacySignature(returns = "LOGICAL", type = Type.METHOD, name = "Remove")
     @LegacyResourceSupport(supportLvl = CVT_LVL_FULL | RT_LVL_FULL)
     public logical remove()
     {
        return function(ArrayIterator.class, this, "Remove", logical.class, new Block((Body) () ->
        {
           if (_isGreaterThanOrEqual(new decimal(EnvironmentOps.getVersion()), decimal.fromLiteral("12.0")))
           {
              Assert.notZero(currentExtent, new character("Current iterator position"));
           }
           else
           {
              Assert.notZero(currentExtent, new character("Extent"));
           }
           returnNormal(iteratedArray.ref().remove(iteratedArray.ref().getValue(currentExtent)));
        }));
     }
  

  
  The above was necessary for other tests to pass so not sure where to add the lead attributes error..

At the moment resolving the proversion at conversion time is broken as it tries to use EnviromentOps.getVersion that reads from directory. Constantin is aware of this.

We should not use EnviromentOps.getVersion() at conversion time. I think we need an alterate approach there which reads from the p2j.cfg.xml.

I added the Array.cls and ArrayIterator.cls from github ADE, where they are the same for 12.8 and 11.7 release.
My unit tests with v12 are passing but with v11 they are not. This means that the implementation of these classes is different in 11.7 even if on github they have no differences.

Ioana-Cristina Prioteasa wrote:

I added the Array.cls and ArrayIterator.cls from github ADE, where they are the same for 12.8 and 11.7 release.
My unit tests with v12 are passing but with v11 they are not. This means that the implementation of these classes is different in 11.7 even if on github they have no differences.

For now, please document at the FWD implementation of these classes that the 11.7 support is based on the OpenEdge 11.7 release and not ADE, and fix the tests to pass using the OpenEdge default install for these.

• Progress.IO.ByteOrder at tests/oo/progress/io/TestByteOrder.cls : testcases rev. 1730
• Progress.IO.MemoryOutputStream at tests/oo/progress/io/TestMemoryOutputStream.cls: testcases rev. 1730
• Progress.Collections.List at tests/oo/progress/collections/TestList.cls: testcases rev. 1744
• HASH-CODE built in function at tests/base_language/builtin_functions/functionality/TestHash-Code.cls: testcases rev. 1744
• OpenEdge.Core.Collections.ArrayIterator at tests/oo/openedge/core/collections/array_iterator/: testcases rev. 1748

Rebase notification

Task branch 9436b was rebased with trunk rev. 15941. Did not encountered conflicts related to functionality.

Thanks, Paul. Some history entries were 'mixed up' during rebase - please do a bzr diff -r 15941 --using meld and fix them. Also some files have the history number twice.

I'll put this into conversion testing.

I've ordered the history entries based on the date is this what are you referring to?

Paul Bodale wrote:

I've ordered the history entries based on the date is this what are you referring to?

Yes, the order is not 'by date', is by 'when the change was made merged'. So ICP's change is made in this branch, and PBB's is in trunk - then PBB's history entry must be before ICP's change.

Constantin Asofiei wrote:

Yes, the order is not 'by date', is by 'when the change was made merged'. So ICP's change is made in this branch, and PBB's is in trunk - then PBB's history entry must be before ICP's change.

Thanks for pointing that out! Committed rev. 15955 that addresses this issue.

Harness passed with 9436b

Smoke testing of another GUI customer app passed with 9436b. ✅