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

Enhancement/test container reuse #2883

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tharoldD
Copy link

@tharoldD tharoldD commented Nov 5, 2023

When using the Axon test container, if you set the reuse flag to true, the current setup always tries to initialize the container contexts, we should check if reuse has been set to true and also if the container has already been initialized before re-initializing

@smcvb smcvb added Type: Enhancement Use to signal an issue enhances an already existing feature of the project. Priority 3: Could Low priority. Issues that are nice to have but have a straightforward workaround. Status: In Progress Use to signal this issue is actively worked on. labels Nov 6, 2023
@smcvb smcvb added this to the Release 4.9.1 milestone Nov 6, 2023
@smcvb smcvb requested review from a team, gklijs, CodeDrivenMitch and smcvb and removed request for a team November 6, 2023 11:06
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

That's a nice little addition you've done for us, @tharoldD. Had to check on the isShouldReuse flag (ending up here), but this seems right to me.

@tharoldD
Copy link
Author

tharoldD commented Nov 7, 2023

@smcvb do i need to do anything about the failing checks?

@abuijze
Copy link
Member

abuijze commented Nov 7, 2023

I am wondering if it isn't easier to use the AXONIQ_AXONSERVER_STANDALONE environment variable to automatically initialize the AxonServer node for standalone mode. That way, Axon Server will automatically initialize if that hasn't been done yet.

@tharoldD
Copy link
Author

tharoldD commented Nov 7, 2023

@abuijze does using the env variable negate the need to call the initCluster method?

@abuijze
Copy link
Member

abuijze commented Nov 8, 2023

Yes, it should. I think the con te ainer uses the init method as that has always been the way for the Enterprise Edition. But since the two versions are merged into a single deliverable (v2023.0), we have the environment variable to do that as well.

@smcvb
Copy link
Member

smcvb commented Nov 8, 2023

@smcvb do i need to do anything about the failing checks?

The issue in the JDK17 build also happens on our automated dependabot PRs.
So, I am investigating the predicament on that one.

The issue with the JDK21 build comes from time-based tests.
Axon Framework has quite some of those, which cause the tests to fail from time to time.
Especially on the machines that run the test, sadly.
So, again, no task for you. I am retriggering the JDK21 build until it succeeds or until I conclusively find an issue.

@smcvb
Copy link
Member

smcvb commented Nov 8, 2023

Although the AXONIQ_AXONSERVER_STANDALONE suggestion doesn't align with your original intent I assume, if you are up for the changes @tharoldD, we'd gladly review those too!

@smcvb
Copy link
Member

smcvb commented Nov 8, 2023

I have just uncovered why the SpringBootDockerComposeIntegrationTest fails within a GHA and not locally, based on this issue.
It has to do with the Docker Compose version used that's incremented by GitHub but doesn't align with Spring Boot's expecations at the moment.
This comment issue on the Spring Boot project explains it will be resolved in the release later this month.

Normally I would find a workaround ASAP to be sure the JDK17 and (as it turns out) JDK21 processes work as intended.
However, as the team is moving their focus towards Axon Framework 5, thus we are not providing a lot of features or enhancement to 4.x.x, I'll be patient until dependabot updates the Spring Boot dependency to 3.1.6.
Furthermore, I will make it a point to validate the JDK17 and JDK21 build locally once we've come around to resolving your PR, @tharoldD.

Long story short: you don't have to do anything about the failing JDK17 and JDK21 builds.
I hope the above makes our approach clear on the subject.

@smcvb smcvb modified the milestones: Release 4.9.1, Release 4.10.0 Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 3: Could Low priority. Issues that are nice to have but have a straightforward workaround. Status: In Progress Use to signal this issue is actively worked on. Type: Enhancement Use to signal an issue enhances an already existing feature of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants