Update the library versions used of dependencies used by the build #1291
Conversation
|
The build works for me locally, but I run it with a Java 17 JVM. So I think that's why the Javadoc generation fails: So I think this needs to use Java 17:
|
|
I guess it's this that needs to change:
But I think that even when I change this, for security reasons, the action won't use my version. I can try but I doubt that will work... |
|
Actually maybe that security measure is not in place because it does seem to have worked:
|
|
The build part worked. Locally I let the build run the tests and just use "clean verify", but in this thing uses This looks like the fatal part of the failure to launch the test:
The test itself is successful in the workspace:
I'm not sure what to make of this. I've tried to run all the tests locally in the Tycho build, but my IDE keeps getting dead locked when I do that... |
|
My local build for that test is like this:
|
|
Before I thrash too much more on this, I'll wait for some comments. Perhaps you've seen issues like this before... |
|
Ed, reviving the tests was quite an effort, so we stopped running all tests and added each fixed suite one by one in the way you see us do now. We are executing all tests we have fixed, but I don't know if these are all the available tests. |
|
There are many more tests than just the ones being run. My latest PR disables all tests by default skipTests=true and enables only those tests that were currently running, plus I enabled org.eclipse.birt.core.tests after fixing a few failures. For me locally, all the tests that were previously running do now also run and pass locally as part of a single mvn invocation. Let's keep our fingers crossed that this works exactly the same on the builder server. |
|
It looks like I did it correctly. All the tests that previous ran still run and pass. But I don't know if you publish some details about the tests results. You have to look in the log to see that they did actually run and pass. Hopefully we can move forward from this without a need to rebase! |
|
@merks I read your mail with the comment of: // diagonal & antidiagonal special function and the usage of openRootTag(). Please be aeware that is not wrong by default. Because the function add a special HTML-JavaScript for the header. So the call was figured out on my research. If you can see in the source before the openRootTag() is called twice and this is part of the original implementation (before my cell diagonal change was done). So if you change it then you it would be good if the functionality of the added script is given already. |
|
With the local changes (not yet in this PR) this is the actual result from the emitter for the test case: The indentation of is off because at some point not that long ago the logic to close the meta tag was made conditional. I added the CDATA stuff like the other methods used. Does that look correct? |
Update the setup to provide a functional targlet task that generates the org.eclipse.birt.target.target to match. Do not use any repository reference for the build but rather use purely the *.target. Use plugin-source's feature-source for feature source generation and ensure now warnings about plugins/features without sources. Reduce noise from Javadoc generation. Ensure that skipTests is the default and enable those tests that are currently running in the builds and also enable many of the other tests that were not currently running and fix tests failing with the latest libraries and versions. eclipse-birt#1286
|
The build runs these 17 tests now:
|
|
Cheers, Ed! That is awesome. |
|
It's ready to commit as far as I'm concern. Does anything speak against that? |
|
Thanks! I will now focus on what needs to be promoted and how best to manage that... |
|
Your example looks good. The meta tag is open tag given through the standard but based on history it was necessary to close them. I think also the CDATA-fragments are old code because the part are commented out. |
|
It's very weird not to close it, but there was a comment about a bugzilla relative to that code being conditional. But it looks ugly and seems wrong to me... I'm not sure what you mean by "parts are commented out." It's a "standard pattern" to use such "brackets" around Javascript content: https://stackoverflow.com/questions/66837/when-is-a-cdata-section-necessary-within-a-script-tag |
|
Yes, the meta-tag looks wrong but it is right. The current standard of HTML allow that. If there would be the xhtml stanard would be used then it would be an empty tag with "/" like "<meta ... />" but HTML5 doesn't like in every case the xhtml syntax. And I mean the CDATA is commented: if you execute your example on a browser the CDATA will be ignored because it is commented with "//" but if you use it in an xml-parse process it is necessary that the content of CDATA will not be parsed. Therefore you will not the such script-start-sequence by default on the HTML-pages (but necessary for the xml-parsing process). |
|
I can't parse "you will not the such script-start-sequence" so I'm not sure what you are trying to say. There isn't a problem is there? |
|
You can go ahead with your changes, all is fine. I tried to say
If you take a look into browser-dev-tools you won't be see "...//<
|
I have done the fix under #1305 - HTMLReportEmitter. I have corrected the method call sequence which is now correct like before. |
|
I can help with the failing test tomorrow. |
|
I improved the emitter and the test case was running sucessfully for the new PR. |
Update the setup to provide a functional targlet task that generates the org.eclipse.birt.target.target to match.
Do not use any repository reference for the build but rather use purely the *.target.
Use plugin-source's feature-source for feature source generation and ensure now warnings about plugins/features without sources.
Reduce noise from Javadoc generation.
#1286