Open eclipse-qvt-oml-bot opened 1 day ago
By Ed Willink on May 07, 2020 03:57
Surely the test should emulate what a user does? Presumably the user develops a *.java black box in some source folder that is built by the Java builder. Then the user runs QVTo and exploits the black box from the classpath.
I see little difference to a normal Java class that is used in a Java application.
The only difference is that 'live' tooling such as the QVTo editor may need a URLClassLoader to load from the project bin folder rather than the plugin JAR. And of course the class must be reloaded after a change.
By Christopher Gerking on May 07, 2020 04:38
(In reply to Ed Willink from comment #51)
Surely the test should emulate what a user does? Presumably the user develops a *.java black box in some source folder that is built by the Java builder. Then the user runs QVTo and exploits the black box from the classpath.
Right.
The only difference is that 'live' tooling such as the QVTo editor may need a URLClassLoader to load from the project bin folder rather than the plugin JAR.
Exactly. Thus to ensure a reasonable test coverage, the class loading from the bin folder should be part of the test. But if the blackbox can already be loaded from the plugin JAR, this part is never tested. To be precise, a prerequisite is that Class.forName(...) does not produce a result.
By Ed Willink on May 07, 2020 10:12
If the blackbox is in perhaps its own plugin that is referenced from tests/pom.xml so that it is built, but is not referenced from the test plugin, it will not be on the test plugin classpath, so it cannot be loaded without the help of your URLClassLoader.
By Christopher Gerking on May 08, 2020 08:46
(In reply to Ed Willink from comment #53)
If the blackbox is in perhaps its own plugin that is referenced from tests/pom.xml so that it is built, but is not referenced from the test plugin, it will not be on the test plugin classpath, so it cannot be loaded without the help of your URLClassLoader.
The problem is that the URLClassLoader is currently restricted to the project's bin folder, which cannot be set to an external location outside the project (JavaModelException). But of course we could programmatically copy the class file into the project.
By Ed Willink on May 08, 2020 09:16
Have a look at org.eclipse.ocl.examples.codegen.dynamic.ExplicitClassLoader then which loads classes that are in a completely synthetic tsst project.
One possible hazard is to ensure that if your TestClass references a QVToUtilityClass that is part of the regular operation, it must use the regular class loader to avoid two classes. ExplicitClassLoader falls back to the regular class loader for whatever it doesn't handle.
By Ed Willink on May 21, 2020 05:57
After rebasing on master (branch ewillink/472482) and doing a local Build QVTo nightly, the bug472482 fails with a class version error.\ [INFO] Command line:\ [C:\Program Files\Java\jdk1.8.0_192\jre\bin\java.exe, -Dosgi.noShutdown=false, -Dosgi.os=win32, -Dosgi.ws=win32, -Dosgi.arch=x86_64, -Xms128m, -Xmx1G, -ea, -Dosgi.clean=true, -jar, C:\Users\Edward.m2\repository\p2\osgi\bundle\org.eclipse.equinox.launcher\1.5.700.v20200207-2156\org.eclipse.equinox.launcher-1.5.700.v20200207-2156.jar, -data, E:\GIT\org.eclipse.qvto\tests\org.eclipse.m2m.tests.qvt.oml\target\work\data, -install, E:\GIT\org.eclipse.qvto\tests\org.eclipse.m2m.tests.qvt.oml\target\work, -configuration, E:\GIT\org.eclipse.qvto\tests\org.eclipse.m2m.tests.qvt.oml\target\work\configuration, -application, org.eclipse.tycho.surefire.osgibooter.uitest, -testproperties, E:\GIT\org.eclipse.qvto\tests\org.eclipse.m2m.tests.qvt.oml\target\surefire.properties]\ May 21, 2020 9:49:24 AM java.util.prefs.WindowsPreferences WindowsRegOpenKey1\ WARNING: Trying to recreate Windows registry node Software\Classes\eclipse+command at root 0x80000001.\ May 21, 2020 9:49:24 AM java.util.prefs.WindowsPreferences WindowsRegOpenKey1\ WARNING: Trying to recreate Windows registry node Software\Classes\eclipse+command\shell\open\command at root 0x80000001.\ Running org.eclipse.m2m.tests.qvt.oml.AllTests\ Tests run: 2093, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 566.471 s <<< FAILURE! - in org.eclipse.m2m.tests.qvt.oml.AllTests ...\ runTestbug472482 Time elapsed: 0.369 s <<< ERROR!\ java.lang.UnsupportedClassVersionError: org/eclipse/m2m/tests/qvt/oml/Blackbox has been compiled by a more recent version of the Java Runtime (class file version 57.0), this version of the Java Runtime only recognizes class file versions up to 52.0\ at org.eclipse.m2m.tests.qvt.oml.TestQvtParser.compile(TestQvtParser.java:401)\ at org.eclipse.m2m.tests.qvt.oml.TestQvtParser.runTest(TestQvtParser.java:288)
runTestscr878 Time elapsed: 0.655 s
It was launched with Java 8 which is what most of my configs use. But somehow the blackbox was compiled with Java 13 which isn't even installed on my machine. I guess you compiled it on yourt machine then committed a class file to GIT. Not acceptable.
By Ed Willink on May 21, 2020 06:04
Adjusting the classpath so that /org.eclipse.m2m.tests.qvt.oml/parserTestData/sources/bug472482/src is a source folder ensures that the JDT builder builds the class, but an appropriate magic incantation is needed to tell Tycho/Maven that this dark corner needs to be copied in some way. I'm not clever enough.
The problem seems to be entirely in the horrible test infrastructure.
The user scenario is that they incorporate the BlackBox.java as an ordinary class in an ordinary plugin that is built in a very ordinary way. Let the test do the same. Surely org.eclipse.m2m.qvt.oml.examples.blackbox or something similar could host all the blackbox sources that are needed for testing so that they are compiled normally and referenced normally?
No need for horrible classpath magic for JDT and Maven/Tycho.
By Christopher Gerking on May 22, 2020 09:35
(In reply to Ed Willink from comment #56)
I guess you compiled it on yourt machine then committed a class file to GIT.
Right, that's the root of all evil. I'm pretty sure there is a much better solution to this problem, will have a look next month.
May 26, 2020 16:47
New Gerrit change created: https://git.eclipse.org/r/163639
May 29, 2020 03:26
New Gerrit change created: https://git.eclipse.org/r/163817
By Ed Willink on May 29, 2020 03:54
(In reply to Eclipse Genie from comment #60)
New Gerrit change created: https://git.eclipse.org/r/163817
This just cherry picks the single commit with extra catches onto master without the preceding not yet debugged extra test. It passes tests but when installed in the hope that it might make the error log entry vanish, it actually makes matters worse. There are now two similar error log entries on different Xtext files per-build.
By Christopher Gerking on May 29, 2020 06:27
(In reply to Ed Willink from comment #61)
There are now two similar error log entries on different Xtext files per-build.
What does the error message look like? Failed to load Java black-box unit 'org.eclipse.m2m.tests.qvt.oml.bbox.InvalidLibrary' ?
By Ed Willink on May 29, 2020 07:02
(In reply to Christopher Gerking from comment #62)
What does the error message look like? Failed to load Java black-box unit 'org.eclipse.m2m.tests.qvt.oml.bbox.InvalidLibrary' ?
Essentially the same. The new message comes from a fullBuild whereas the old comes from an incrementalBuild.
By Ed Willink on May 29, 2020 09:02
Going in with a debugger, I am unconvinced that I successfully installed the build with the cherry pick.
But when trying to debug I see a new confusion; Bug 563721.
It appears that what started as an irritating warning has migrated to an irritating error log entry with some seriously dubious functionality revealed along the way.
@Chris: Given that we should aim to be done by RC1 on Tuesday (4 days time) and that this is not your top priority, I see no way that the significant underlying rationalization and testing will be done in time.
Will reverting all Bug 472482 commits get us back where we started or do we need to go all the way back to 3.10.1 deferring 3.10.2 till 2020-09?
By Christopher Gerking on May 29, 2020 13:48
(In reply to Ed Willink from comment #64)
@Chris: Given that we should aim to be done by RC1 on Tuesday (4 days time) and that this is not your top priority, I see no way that the significant underlying rationalization and testing will be done in time.
I agree, I can spend much more time on this issue from mid-June on.\
Will reverting all Bug 472482 commits get us back where we started?
I'm pretty sure.
By Ed Willink on May 29, 2020 16:27
(In reply to Christopher Gerking from comment #65)
Will reverting all Bug 472482 commits get us back where we started? I'm pretty sure.
Actually only one commit, which #c33 suggested was trivial so I was perhaps too enthusistic aboout incldding it. REverting and we seem to be back to a warning.
It seems we have a rubbish legacy proprietary blackbox approach. The QVTo specification is very thin mandating a proprietary approach.
Let's do it right leaving the legacy rubbish for legacy.
I see that the problem is that in a *.qvto I have...
library BlackBoxLibrary;
blackbox helper getURI(eObject : EObject) : String;
and need to map the library name "BlackBoxLibrary" to the class "org.eclipse.ocl.examples.build.qvto.BlackBoxLibrary".
The rubbish approach is an extension point that is bad in an Eclipse project and even worse standalone.
<extension point="org.eclipse.m2m.qvt.oml.javaBlackboxUnits">\
<unit name="ExampleJavaLib" namespace="m2m.qvt.oml">\
<library name="BlackBoxLibrary" class="org.eclipse.ocl.examples.build.qvto.BlackBoxLibrary">\
<metamodel nsURI="http://www.eclipse.org/emf/2002/Ecore"/>\
</library>\
</unit>\
</extension>
No idea what the unit/namespace/metamodel clutter is.
If we add a library sub-clause
library BlackBoxLibrary implementedBy 'org.eclipse.ocl.examples.build.qvto.BlackBoxLibrary';
we know exactly what to do in Eclipse or standalone. Old code can remain crazily complicated and inefficient. New code can do it right directly.
(If we really want a mapping level from internal names to external facilities we should have a regular facility that works standalone/in-Eclipse and amortises the plugin extensiin point, the .settings/org.eclipse.m2m.qvt.oml.mmodel.urimap and .. and .. and of course is compatible with OCL, ATL, Epsilon, ...)
May 29, 2020 18:37
New Gerrit change created: https://git.eclipse.org/r/163875
By Christopher Gerking on May 29, 2020 18:56
(In reply to Ed Willink from comment #66)
Actually only one commit, which #c33 suggested was trivial so I was perhaps too enthusistic aboout incldding it.
My mistake. I should have been aware of all the things that can go wrong.
However I managed to get rid of the errors in org.eclipse.ocl.examples.build, at least on my machine. The remaining problem was just that load errors were reported even though the blackbox was not explicitly imported. Clearly, reporting errors makes sense only if there is a definite import.
Commit ID: 0a123dcf42e1754a1d11ae14e7967f41822487a5
No idea what the unit/namespace/metamodel clutter is.
Analogously to a QVTo unit, which may comprise multiple transformations or libraries, you can also merge multiple Java classes into a shared blackbox unit. Almost never makes sense. When converting Java methods into QVTo operations, the 'metamodel' part indicates the namespace URIs on which a blackbox relies. The EClasses in these namespaces are the ones against which the return/parameter types of the Java methods are resolved.
I see that the problem is that in a *.qvto I have...
library BlackBoxLibrary;
blackbox helper getURI(eObject : EObject) : String;
and need to map the library name "BlackBoxLibrary" to the class "org.eclipse.ocl.examples.build.qvto.BlackBoxLibrary".
But that's basically the same thing that you can do today, or at least with the above patch. In your BlackBoxLibrary.qvto, just add
import org.eclipse.ocl.examples.build.qvto.MyJavaBlackBoxLibrary;
If you import at least one Java class directly, only the imported ones will be scanned for implementations of your blackbox operations. For this to work, it is crucial that the Java class is named differently than the QVTo file, otherwise there will be a circular import. That's why I assume the Java class to be renamed to MyJavaBlackBoxLibrary.
Another minor thing is the order of builders in org.eclipse.ocl.examples.build.qvto. I don't know how much effect it actually has, but maybe changing the build order to Java first, QVTo afterwards will show better results.
By Ed Willink on May 30, 2020 03:55
(In reply to Christopher Gerking from comment #68)
import org.eclipse.ocl.examples.build.qvto.MyJavaBlackBoxLibrary;
This is a non-standard construct since org.eclipse.ocl.examples.build.qvto.MyJavaBlackBoxLibrary is not a QVTo construct rather a Java declaration. As such it shioult at least be in quotes, and perhaps preceded by java:
it is crucial that the Java class is named differently than the QVTo file
It is unfortunate that the only blackbox I have ever attempted violates this simple rule. I suspect that most one-BB developers will hit exactly the same inexcusable problem. There are three namespaces QVTo, file, Java. Filenames may be restricted a bit but QVTo and Java namespaces must be independent.
No idea what the unit/namespace/metamodel clutter is. Analogously to a QVTo unit, which may comprise multiple transformations or libraries, you can also merge multiple Java classes into a shared blackbox unit. Almost never makes sense. When converting Java methods into QVTo operations, the 'metamodel' part indicates the namespace URIs on which a blackbox relies. The EClasses in these namespaces are the ones against which the return/parameter types of the Java methods are resolved.
I see no reason why non-trivial Java blackboxes might not be aggregated into a single 'signature' QVTo file: e.g.
modeltype Ecore "strict" uses ecore('http://www.eclipse.org/emf/2002/Ecore');
import pkge.Class1;\ import pkge.Class2;
library Class1;
helper class1method(...)
library Class2;
helper class2method(...)
The modelType declaration provides the metamodel name visibility, nothibg is needed in extension points.
But I suggest we add the much simpler
modeltype Ecore "strict" uses ecore('http://www.eclipse.org/emf/2002/Ecore');
library Class1 implementedBy 'java:pkge.Class1';
helper class1method(...)
library Class2 implementedBy 'java:pkge.Class2';
helper class2method(...)
This isolates the QVTo and Java namespaces at the Namespace level; only method signatures are expected to correlate by method name and parameter/return type between preceding library declaration and associated implementedBy.
Any metamodel names mentioned in the helper signatures are resolved as normal by the modeltype declaration. If the Java code uses different metamodels then a signature check between QVTo and Java variants of the shared helper signature should diagnose the problems at edit time.
Jun 10, 2020 10:32
New Gerrit change created: https://git.eclipse.org/r/164641
By Christopher Gerking on Jun 10, 2020 11:32
(In reply to Ed Willink from comment #57)
Adjusting the classpath so that /org.eclipse.m2m.tests.qvt.oml/parserTestData/sources/bug472482/src is a source folder ensures that the JDT builder builds the class, but an appropriate magic incantation is needed to tell Tycho/Maven that this dark corner needs to be copied in some way. I'm not clever enough.
So that part was actually easy, the src folder just required a separate bin folder within the bug472482 directory. In addition, it was easily possible to exclude that bin folder from the runtime classpath, thereby ensuring that the class gets loaded by the URLClassLoader under test.
Commit ID: 2b01d521b648748bfebc4e0c497af12d269406a1
Jun 17, 2020 05:27
New Gerrit change created: https://git.eclipse.org/r/165036
By Christopher Gerking on Jun 18, 2020 09:48
Improved the error handling in commit cbb66894e495c2d93d2ce00a12cc815e1985b124. First, load errors should only be reported when importing a Java blackbox explicitly. Second, the additional errors from comment 21 are now handled as well.
Jun 18, 2020 10:29
New Gerrit change created: https://git.eclipse.org/r/165165
By Christopher Gerking on Jun 18, 2020 11:24
Improved the unloading of blackbox classes in commit 9e0cd28560a92bc0b70a7f8854a406d5204691ba. Blackboxes are now unloaded when cleaning a Java project and also after each individual parser test.
Jul 08, 2020 11:04
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166003
By Christopher Gerking on Jul 08, 2020 11:22
(In reply to Ed Willink from comment #34)
As public API, it deserves a Javadoc comment, particularly in regard to the null return.
As nothing to do with QvtOperationalVisitorCS, perhaps there is a better place to put it, perhaps a CSUtils class or perhaps in the QvtOperationalModuleEnv argument type.
Done. Commit ID: 475394b2cc56f49641412c1ce4814b26963d4a78
Are there any objections left to merging into master?
By Ed Willink on Jul 09, 2020 04:52
A rebase on master and a successful build and tist is essential.
Your branch is at least 5 commits stale. I've rebased it a few times in the past to comment. I'm inclined to wait till you provide a rebased version before reviewing where we have got to.
Unfortunately my latest commit to eliminate platform deprecations cannot be pushed yet; see Bug 565077
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166049
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166050
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166051
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166052
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166053
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166054
Jul 09, 2020 10:47
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166055
By Christopher Gerking on Jul 09, 2020 10:58
Maybe I rebased the wrong way round? If so, sorry about that.
By Ed Willink on Jul 09, 2020 12:29
Yet another reason not to use Gerrit.
It seems to be designed to support a single commit contribution that can be reviewed in one go. If you don't flatten, then everything happens many times, including a separate build for each commit (since it needs to identify which commit broke the build).
So much better to use the old fashioned approach of a branch, which would allow me to see what you are doing much more easily, and if you want to do a remote build, use the qvto-branch-tests job.
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166063
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166064
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166065
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166066
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166067
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166068
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166069
Jul 09, 2020 14:14
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166070
By Christopher Gerking on Jul 09, 2020 14:21
(In reply to Ed Willink from comment #87)
Yet another reason not to use Gerrit.
It is still not clear to me how to bypass Gerrit. My remote configuration is the default ssh://user_id@git.eclipse.org:29418/mmt/org.eclipse.qvto.git.
By Ed Willink on Jul 09, 2020 14:42
You create a branch called e.g. cgerking/472482 and commit to it then occasionally push to origin and build by specifying cgerking/472482 to the qvto-branch-tests job.
I can easily see and review cgerking/472482
When appropriate and after rebasing on master, cgerking/472482 can be FAST merged to master, which can also be pushed to origin. It will be auto-built within 6 hours, except on Sundays when a single full build is imposed.
Jul 10, 2020 05:40
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166112
Jul 10, 2020 05:40
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166113
Jul 10, 2020 05:40
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/166114
| --- | --- | | Bugzilla Link | 472482 | | Status | REOPENED | | Importance | P3 normal | | Reported | Jul 13, 2015 05:56 EDT | | Modified | Feb 19, 2021 13:07 EDT | | Depends on | 563721 | | Blocks | 563596, 453909 | | See also | Gerrit change https://git.eclipse.org/r/138084, Gerrit change https://git.eclipse.org/r/162063, Gerrit change https://git.eclipse.org/r/162069, Gerrit change https://git.eclipse.org/r/162548, Gerrit change https://git.eclipse.org/r/163639, Gerrit change https://git.eclipse.org/r/163817, Gerrit change https://git.eclipse.org/r/163875, Gerrit change https://git.eclipse.org/r/164641, Gerrit change https://git.eclipse.org/r/165036, Gerrit change https://git.eclipse.org/r/165165, Gerrit change 166003, Gerrit change 166049, Gerrit change 166050, Gerrit change 166051, Gerrit change 166052, Gerrit change 166053, Gerrit change 166054, Gerrit change 166055, Gerrit change 166063, Gerrit change 166064, Gerrit change 166065, Gerrit change 166066, Gerrit change 166067, Gerrit change 166068, Gerrit change 166069, Gerrit change 166070, Gerrit change 166112, Gerrit change 166113, Gerrit change 166114, Gerrit change 166115, Gerrit change 166116, Gerrit change 166117, Gerrit change 166118, Gerrit change 166119, Gerrit change 166135, Gerrit change 166136, Gerrit change 166137, Gerrit change 166138, Gerrit change 166335, Gerrit change 166336, Gerrit change 166337, Gerrit change 166338, Gerrit change 166339, Gerrit change 166340, Gerrit change 166341, Gerrit change 166342, Gerrit change 166343, 387637, Gerrit change 176315, Gerrit change 176316, Git commit 66e8aab0, Git commit db5b0c81 | | Reporter | Dennis Hendriks |
Description
In some QVTo project, I have a Java class named 'TimestampLibrary', with an @Module annotation:
[code]\ @Module(packageURIs={"http://www.eclipse.org/emf/2002/Ecore"})\ public class TimestampLibrary { ...\ }\ [/code]
It has a method:
[code]\ public long getMicros(Date date, int micros) { ...\ }\ [/code]
The library class is registered in plugin.xml of that same project:
[code]\