-
Notifications
You must be signed in to change notification settings - Fork 325
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
ci(ITs): support JBoss ITs with GitHub secrets #3469
Conversation
name: JBoss integration tests | ||
runs-on: ubuntu-latest | ||
needs: build | ||
if: github.event_name != 'pull_request' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only happen when a non-forked pull request.
@v1v I've split the jboss integration tests (and changed the profile name for consistency), so with that we should already be able to execute them in a separate build step. |
…ent-java into feature/run-jboss * 'feature/run-jboss' of https://github.com/elastic/apm-agent-java: add latest jboss version just in case
@@ -320,3 +320,35 @@ jobs: | |||
path: | | |||
**/junit-*.xml | |||
**/TEST-*.xml | |||
|
|||
jboss: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor] we could make that step a sub-step of the application server tests,but as it's quite fast (about 6min) in CI that's fine for now and allows us to keep an eye on it for now in case of failure.
@@ -270,16 +270,16 @@ public void start() { | |||
log.info("starting container with {} = {}", jvmEnvironmentVariable, value); | |||
} | |||
|
|||
// log app server output for easier debugging | |||
withLogConsumer(new Slf4jLogConsumer(log)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was required because otherwise the container output wasn't captured
.waitingFor(new HttpWaitStrategy().forPort(8080).forStatusCode(200)); | ||
|
||
if (preserveDefaults) { | ||
// for older versions setting JAVA_OPTS means overwriting the defaults |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only needed for some of the older jboss versions, the latest ones are able to deal with customized JAVA_OPTS variable.
What does this PR do?
Support specialised ITs that require access to GitHub action secrets.
It requires support for the new maven profile called
ci-jboss-integration-tests
, so all the ITs that require access to the JBoss docker images are disabled by default.Docker images are not yet available in the internal docker registry, that will happen eventually.
Checklist