Skip to content

Conversation

@tomjenkinson
Copy link
Member

@tomjenkinson tomjenkinson commented Jan 8, 2026

CORE AS_TESTS

JDK21

AS_BRANCH=testcontainer

(following up on #2460)

@jmfinelli
Copy link
Contributor

I would be happy to add the removing of the caching (I'll need to look to what that means in practice :) ). Do you agree with the general reason for the need to use a dist/ build though? In that the CORE runner doesn't (at least predictably) have (e.g.) the GlassFish JSF jar when running the WildFly from the as_builder job because the CORE runner did not download the server dependencies in it's Maven build? I am wondering if the cache that you mentioned might be expected to be having that Jar? But then why the CORE runner didn't find that kind of Jar to be able to run the wildfly build/ version?

I understand why you would like to use WildFly from the dist folder instead of the one from the build folder. However, I went for the build WildFly because the overall size of the zip and related artifacts is smaller than the size of the dist WildFly. Although GHA is free, they have space limits when it comes to uploading artifacts. Even though the lifespan of those uploads is limited to 1 day, we could hit those limit if we have more users testing their PRs in parallel.

@jmfinelli jmfinelli force-pushed the tomjenkinson-patch-1 branch from 5bafa9d to 279cf45 Compare January 8, 2026 12:27
@jmfinelli
Copy link
Contributor

@tomjenkinson I tried to introduce a step to delete the WildFly dist artifact once all depending axes are done...let's see if it works

@jmfinelli
Copy link
Contributor

Good news number 1:
I was wrong on the size of the dist WildFly. Apparently, the upload-artifact plugin is able to compress the dist zip as efficiently as the build zip. I must have compared the size of the two zips (plus m2 artifacts) locally. Sorry about the confusion.

Good news number 2:
The extra step to clean the WildFly artifact worked like a charm! I will improve this step to delete Narayana artifacts as well.

There is also a bed news unfortunately...CORE failed :-(
I will have a look at it later tomorrow

@tomjenkinson
Copy link
Member Author

Thank you, Manuel! I think in terms of the CORE failure it's maybe understandable in that CORE is running some AS testing too. I can actually see an argument to possibly not run those tests in CORE maybe running them in AS_TESTS instead (if indeed they don't effectively turn out to be done in there anyway). I'm going to add a commit to move that testing and include AS_TESTS in the description of this pull so it will test it. It should be checked if these are not being ran anyway in AS_TESTS in which case maybe this part of the script just needs deleting if we can't find/recall a good reason to have it in CORE again and fix it some other way...

@jmfinelli
Copy link
Contributor

Thank you, Manuel! I think in terms of the CORE failure it's maybe understandable in that CORE is running some AS testing too. I can actually see an argument to possibly not run those tests in CORE maybe running them in AS_TESTS instead (if indeed they don't effectively turn out to be done in there anyway). I'm going to add a commit to move that testing and include AS_TESTS in the description of this pull so it will test it. It should be checked if these are not being ran anyway in AS_TESTS in which case maybe this part of the script just needs deleting if we can't find/recall a good reason to have it in CORE again and fix it some other way...

Thanks Tom. IIRC, I set env. vars one by one in most GHA workflows. As a result, together with modifying axes in narayana.sh, we should also modify the CORE workflow.

@jmfinelli jmfinelli force-pushed the tomjenkinson-patch-1 branch from e592c64 to b5f0cdb Compare January 8, 2026 14:58
@jmfinelli
Copy link
Contributor

jmfinelli commented Jan 8, 2026

I modified the cleanup step locally. As soon as the current GHAs are finished, I'll force push the modifications

@jmfinelli jmfinelli force-pushed the tomjenkinson-patch-1 branch from b5f0cdb to b295d65 Compare January 9, 2026 10:55
@mmusgrov
Copy link
Member

mmusgrov commented Jan 9, 2026

It should be checked if these are not being ran anyway in AS_TESTS in which case maybe this part of the script just needs deleting if we can't find/recall a good reason to have it in CORE again and fix it some other way...

If you mean why do we need the AS for CORE testing, then it was added for CMR integration testing (com.hp.mwtests.ts.jta.commitmarkable.integration).

@tomjenkinson
Copy link
Member Author

It should be checked if these are not being ran anyway in AS_TESTS in which case maybe this part of the script just needs deleting if we can't find/recall a good reason to have it in CORE again and fix it some other way...

If you mean why do we need the AS for CORE testing, then it was added for CMR integration testing (com.hp.mwtests.ts.jta.commitmarkable.integration).

Not that, I basically mean why do we feel we want to do https://github.com/jbosstm/narayana/blob/main/.github/scripts/narayana.sh#L282-L284 in the CORE profile when the AS_TESTS profile can do https://github.com/jbosstm/narayana/blob/main/.github/scripts/narayana.sh#L282-L284 - I presume transactions and iiop-openjdk module tests are ran then anyway.

@jmfinelli jmfinelli force-pushed the tomjenkinson-patch-1 branch from 41eb17b to eb3cb94 Compare January 13, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants