-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Glassfish 7 nightly bundle failures with standalone concurrency TCK 3.0 #1053
Comments
glassfish discussion thread - https://www.eclipse.org/lists/glassfish-dev/msg01304.html |
@gurunrao GlassFish should now pass using the latest nightly and https://github.com/eclipse-ee4j/glassfish/tree/master/appserver/tests/tck/concurrency |
I started making a first pass update to https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run |
Latest https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run run failed with JBoss Nexus 503 failure. How can we use the downloaded concurrency-tck.zip which may help avoid the 503 error? |
@scottmarlow I see, that's my mistake, I should have excluded the audit jars. JBoss has some kind of protection (domain limiting, bandwidth checks or so) that prevents the Eclipse CI from downloading these. They are referenced by another dependency, but can be excluded. I just did a PR for that to GlassFish. |
I just noticed @gurunrao came to the same conclusion, and must have updated gf7-nightly already. I think the job is running now. |
@arjantijms - We have following runs (using runner - https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/glassfish-runner/concurrency-tck)
PS: As an hack, we replace M5 artifact in maven local repository with GF 7 nightly build download. |
The failures are about empty EJB jars. Not sure what that is about. Don't see those locally. Maybe it's a JDK 11 vs JDK 17 issue? @dmatej any idea? |
There should not be any difference between JDK11 and 17. I will try to reproduce it later, but I have no idea now ... |
@gurunrao can you also try locally with just GlassFish? So checkout GF master and then
When it completes building:
|
The results mentioned at #1053 (comment) are for JDK11 |
https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run/43/artifact/glassfish-runner/concurrency-tck/target/glassfish7/glassfish/domains/domain1/logs/server.log has following exception:
|
I ran with eclipse-ee4j/glassfish#24022 locally ( Concurrency TCK Failures:
|
Results with GlassFish commit 4dfcef3a18ad GF built with JDK11, tests executed with JDK11: One thing I noticed - appserver-cli is executed with system default java executable, it ignores maven settings. But it did not affect anything. Meanwhile @arjantijms noticed some mistake :-) |
Yes, I made a mistake indeed. I locally compiled a dependency where I set the version number wrongly. It should have been RC4, but I kept it on RC3. My local RC3 was therefor different from the one that was published. I just did a PR to GF to rectify it. |
We now have essentially 0 errors with the TCK as deployed by maven, but the downloaded version has 1 error. See: https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run/46/ |
Quick glancing at the code, there is indeed a difference between the download version and the maven version: public void testManagedThreadFactoryDefinitionDefaultsEJB() throws Throwable {
ManagedThreadFactory threadFactory = (ManagedThreadFactory)this.managedThreadFactoryDefinitionBean.doLookup("java:comp/concurrent/EJBThreadFactoryB");
CountDownLatch blocker = new CountDownLatch(1);
CountDownLatch allThreadsRunning = new CountDownLatch(2);
CompletableFuture<Object> lookupTaskResult = new CompletableFuture();
CompletableFuture<Object> txTaskResult = new CompletableFuture();
Runnable lookupTask = () -> {
try {
allThreadsRunning.countDown();
blocker.await(MAX_WAIT_SECONDS * 5L, TimeUnit.SECONDS);
lookupTaskResult.complete((ManagedThreadFactory)this.managedThreadFactoryDefinitionBean.doLookup("java:comp/concurrent/EJBThreadFactoryB"));
} catch (Throwable x) {
lookupTaskResult.completeExceptionally(x);
}
}; other has public void testManagedThreadFactoryDefinitionDefaultsEJB() throws Throwable {
ManagedThreadFactory threadFactory = (ManagedThreadFactory)this.managedThreadFactoryDefinitionBean.doLookup("java:comp/concurrent/EJBThreadFactoryB");
CountDownLatch blocker = new CountDownLatch(1);
CountDownLatch allThreadsRunning = new CountDownLatch(2);
CompletableFuture<Object> lookupTaskResult = new CompletableFuture();
CompletableFuture<Object> txTaskResult = new CompletableFuture();
Runnable lookupTask = () -> {
try {
allThreadsRunning.countDown();
blocker.await(MAX_WAIT_SECONDS * 5L, TimeUnit.SECONDS);
lookupTaskResult.complete(InitialContext.doLookup("java:comp/concurrent/EJBThreadFactoryB"));
} catch (Throwable x) {
txTaskResult.completeExceptionally(x);
}
}; |
CC @njr-11 |
It specifically concerns this commit it seems: The 3.0.0 version at the download location is older, and I guess it should have included this change. It's from the same day. The Maven version is newer, and does include this change. |
I'm not sure how that change was missed. It's expected that the download location and maven won't have identical timestamps because separate Jenkins jobs are used. The intention was for both to have the latest set of changes. |
@njr-11 If that was always the intention, could we say that the copied file was just wrong and correct that by copying the correct file to the download location? @ivargrimstad can we do that? |
If it's allowed by Jakarta/Eclipse processes, I'd be fine with that. Here are some problems that I could see arising from this:
I still don't have a good grasp of all the processes and rules, but I suspect that the 3.0.0 TCK zip on downloads will need to remain, in which case a 3.0.1 TCK zip can correct it. I believe they are planning to do that anyways for a couple of other TCK challenges. |
It's only the license.md file if I'm not mistaken, so in my naive reasoning I'd say taking the Maven file, updating the jar with the correct license, and then uploading it, is all there should be to it. |
Btw additionally, we could do a very quick challenge of the test and then exclude it? In 3.0.1 we can add it again. Maybe that is faster? |
jakartaee/specifications#328 is an example (Platform generated) TCK challenge issue for updating a specification download area to contain a staged TCK and to also update the specification index.md file. jakartaee/specifications#330 is a pull request for just updating the specification index.md file. Guidance copied from https://jakarta.ee/committees/specification/tckprocess:
|
It could confusing to exclude a test in the existing https://download.eclipse.org/jakartaee/concurrency/3.0/concurrency-tck-3.0.0.zip file by updating it, better to release a new 3.0.1 or 3.0.2 TCK with excluded tests. |
As the runner on the CI is already using a separate suite.xml file, it would technically be just a matter of adding the test to that exclude file. It already contains about 10 tests that were successfully challenged after 3.0.0 was released. One more wouldn't make much of a difference, in a technical sense at least. |
@arjantijms - Web Profile run has 17 failures with GF runner (https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/glassfish-runner/concurrency-tck) and JDK 11, CI link for the job - https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run/49/ |
@brideck performed the official run of the Concurrency TCK on Open Liberty. It would not have been on web profile only, because, as you pointed out, a number of the tests run in EJBs to cover interoperability. In automated TCK runs on Open Liberty, EJB is always enabled as well for that same reason. I think this was missed that there ought to be a web profile subset of the TCK for testing on web profile only. Inclusion in web profile was new in EE 10 for Concurrency and no one thought about that aspect. |
@njr-11 We may have to work on that in order to get the web profile releases. tbh, I'm afraid some of the other new TCKs I was involved with also may not have thought about the web profile aspect of things. |
Copying @smillidge for awareness because this sounds like something to correct for the 3.0.1 release. |
Could the Concurrency TCK user workaround the web profile failure by excluding all tests that require EAR deployment when they run on a Web Profile implementation? Quoting from TCK process:
|
@brideck is planning to look into that approach tomorrow on Open Liberty to see if it would allow running in web profile. |
Work around seems like the best approach as excluding all the suites with EAR deployments would needlessly impact full platform testing and I don't think we have time to refactor. Do we need words in the TCK user guide? |
@scottmarlow there's also many test archives that somewhere in the test archive contain a remove EJB. Such test archive contains many tests that don't use that remote EJB. Unfortunately they would all have to be excluded, since the archive that contains them doesn't deploy. |
Ps it IS possible to install an Arquillian archive listener that can remove files (including classes from an archive). This would probably still take a couple of days of work though. |
As I understand the process we can't make major changes to the TCK without reballoting Concurrency so exclusions or work around is the only option. |
An Arquillian "filter" works fully at the integrator side of things, but we would have to test which tests fail if the remote EJB bean is removed, or that maybe not everything fails etc. It's a trial and error thing I guess. |
Took a quick look at this right away today and ran against OL with the WebProfile feature set. The tests you would want to exclude from running are:
I just modified the TestNG suite.xml provided in the TCK to test this out. Everything else in the TCK deployed and ran just fine. |
@brideck for GF we get this:
|
Interesting. Is that because there are EJBs present in all of those apps? OL will just skip processing those if you don't have the appropriate feature set enabled, so the rest of the app starts just fine. That would make my list the set of tests that absolutely require remote EJBs in order to execute, and therefore aren't applicable to WebProfile. It'll take some other intervention to make all the rest work for GF. |
Another workaround is to create a web profile junit5 suite to exclude tests that are not appropriate for the web profile. I did this for restful tck tests in the core profile. The suite class and a test invocation interceptor class to do single method level exclusion can be found here: |
Yes, although not EJBs themselves (the web profile supports them), but specifically EJBs that expose a remote interface view. |
See the org.jboss.arquillian.container.test.spi.client.deployment.ApplicationArchiveProcessor as something to implementation to try to filter the test deployment. |
As per https://www.eclipse.org/lists/cu-dev/msg00411.html the concurrency TCK changes are being worked on still. |
Concurrency TCK GF 7 Web profile results are passing, please refer - https://github.com/eclipse-ee4j/jakartaee-tck/wiki/Jakarta-EE-10.0-TCK-results#date--2022-07-29 |
Glassfish 7 nightly bundle failures with standalone concurrency TCK 3.0
https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run/8/#showFailuresLink
The text was updated successfully, but these errors were encountered: