Skip to content
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

Removed the method CommandLineBasedKeystoreGenerator::getSplitCommand… #203

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

TheMarvelFan
Copy link
Contributor

…NameBasedOnOeratingSystem

Fixes issue #198

Removed the above mentioned method. It was returning "gcsplit" when the "os.name" property was "mac", and "csplit" otherwise.

Since Intersmash supports only Linux execution, I hardcoded the value "csplit" where the method call was returning this value, as it is not supposed to change.

Description

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is self-descriptive and/or documented
  • I have implemented unit tests to cover my changes
  • I tested my code in OpenShift

…NameBasedOnOeratingSystem

Removed the above mentioned method. It was returning "gcsplit" when the "os.name" property was "mac", and "csplit" otherwise.

Since Intersmash supports only Linux execution, I hardcoded the value "csplit" where the method call was returning this value, as it is not supposed to change.
@fabiobrz
Copy link
Member

/test

@fabiobrz fabiobrz self-assigned this Sep 25, 2024
@fabiobrz fabiobrz self-requested a review September 25, 2024 14:34
@fabiobrz
Copy link
Member

Intersmash PR CI check results

Hi @TheMarvelFan, this comment is meant to report the outcome of Intersmash integration tests,
which were triggered by @fabiobrz comment, see #203 (comment).

CI checks job <continuous-testing-umb-listener-for-intersmash-pr-checks#136> FAILED:

  • Intersmash integration tests job <eap-8.x-intersmash-integration-tests-community#64> reported failures.

  • Intersmash integration tests job <eap-8.x-intersmash-integration-tests-products-jboss-eap#74> reported failures.

  • Intersmash integration tests job <eap-8.x-intersmash-integration-tests-products-jboss-eap-xp#80> reported failures.

    Please verify your changes locally or reach out to maintainers in order to investigate the issues.

@TheMarvelFan
Copy link
Contributor Author

TheMarvelFan commented Sep 25, 2024

Hi @fabiobrz,
Can you please elaborate about the failing tests? I have followed all the conventions listed in CONTRIBUTING.md, and I don't understand what the issue is here.

Thanks!

@fabiobrz
Copy link
Member

Intersmash PR CI check results

Hi @TheMarvelFan, this comment is meant to report the outcome of Intersmash integration tests, which were triggered by @fabiobrz comment, see #203 (comment).

CI checks job <continuous-testing-umb-listener-for-intersmash-pr-checks#136> FAILED:

* Intersmash integration tests job <_eap-8.x-intersmash-integration-tests-community#64_> reported failures.

<eap-8.x-intersmash-integration-tests-community#64> reported 1 failure, i.e.:

cz.xtf.core.waiting.WaiterException: 
Following events match condition: <at least one exists>
	2024-09-25T14:49:37Z	Build/wildfly-test-app-1	Build appsint-asfl/wildfly-test-app-1 failed
Filter:	 obj names: [wildfly-test-app.*]
	 messages: [Failed to pull image.*, Build.*failed, Error syncing pod, skipping: failed to.*, FailedMount MountVolume.SetUp.*, Error creating: pods.*is forbidden:.*, .*failed to fit in any node.*, Failed to attach volume.*]
	 after: 2024-09-25T14:48:03Z

	at org.jboss.intersmash.testsuite.provision.openshift.WildflyImageProvisionerTestCase.deploy(WildflyImageProvisionerTestCase.java:57)
...

which is a new one, but unrelated to the changes in this PR. We're investigating it as it happens with the main branch too. 🟡

* Intersmash integration tests job <_eap-8.x-intersmash-integration-tests-products-jboss-eap#74_> reported failures.

<eap-8.x-intersmash-integration-tests-products-jboss-eap#74> reported 1 failure, i.e.:

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <503>.

	at org.jboss.intersmash.testsuite.provision.auto.AutoProvisioningTests.testAppIsLive(AutoProvisioningTests.java:53)

which is an intermittent one, reported as fixed by the subsequent run: <eap-8.x-intersmash-integration-tests-products-jboss-eap#75> ✅

* Intersmash integration tests job <_eap-8.x-intersmash-integration-tests-products-jboss-eap-xp#80_> reported failures.

<eap-8.x-intersmash-integration-tests-products-jboss-eap-xp#80> reported 3 failures, i.e.:

  • org.jboss.intersmash.testsuite.provision.openshift.ProvisionerCleanupTestCase.testProvisioningWorkflowCleanup
cz.xtf.core.http.HttpsException: javax.net.ssl.SSLHandshakeException: Remote host terminated the handshake
	at ...	
  • org.jboss.intersmash.testsuite.provision.openshift.WildflyTargetServerTestCase.deploy
cz.xtf.core.waiting.WaiterException: 
Following events match condition: <at least one exists>
	2024-09-25T15:48:09Z	Pod/wildfly-test-app-1-z7kxz	Failed to pull image "image-registry.openshift-image-registry.svc:5000/appsint-main-qmhx/wildfly-test-app@sha256:380949daddd4b090a19ab332d20f418dc966919bb67ff6d0b851390b8a6a8311": unable to retrieve auth token: invalid username/password: authentication required
  • org.jboss.intersmash.testsuite.provision.openshift.WildflyBootableJarTestCase.deploy
cz.xtf.core.waiting.WaiterException: 
Following events match condition: <at least one exists>
	2024-09-25T15:48:50Z	Build/bootable-jar-1	Build appsint-main-qmhx/bootable-jar-1 failed
Filter:	 obj names: [bootable-jar.*]
	 messages: [Failed to pull image.*, Build.*failed, Error syncing pod, skipping: failed to.*, FailedMount MountVolume.SetUp.*, Error creating: pods.*is forbidden:.*, .*failed to fit in any node.*, Failed to attach volume.*]

All of them are intermittent issue, reported as fixed by the subsequent run: <eap-8.x-intersmash-integration-tests-products-jboss-eap-xp#81> ✅

Based on the above findings, the changes in the PR are deemed to pass the integration tests and can now be reviewed.

@fabiobrz
Copy link
Member

Can you please elaborate about the failing tests?

Sure @TheMarvelFan - what you see here is the result of integration tests which have been triggered by a comment of mine.

The report is posted as a comment made by me, although it is an automated process.

This is a temporary solution which we're testing and improving right now. In theory we aim at using a service account and proper checks. See more details here.

I have followed all the conventions listed in CONTRIBUTING.md, and I don't understand what the issue is here.

Hopefully it's more clear now, but definitely thank you for your feedback, which helps us improve the CI processes and the related documentation.

As you can see above, I've then reviewed the failures in the jobs which are mentioned in the report, and have deemed the changes to pass the integration tests 🙂

Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR and for your first contribution to the Intersmash prject @TheMarvelFan! 🎉

Feel free to keep contributing, by logging issues, working on existing ones, or discuss topics! - we love to get feedback from the community 🙂

Changes LGTM, merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
it-passed lgtm This PR has been approved by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants