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

Fix DB2 integration tests DAT-13113 #4293

Merged
merged 12 commits into from Oct 20, 2023
Merged

Fix DB2 integration tests DAT-13113 #4293

merged 12 commits into from Oct 20, 2023

Conversation

wwillard7800
Copy link
Contributor

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

The Db2IntegrationTest had issues

Things to be aware of

Things to worry about

Additional Context

@StevenMassaro
Copy link
Contributor

Nice work!! Question for you - is it possible to fix this in the testcontainers repo, rather than fix it here? I'm sure there are other people who would like to get these fixes.

@abrackx
Copy link
Contributor

abrackx commented Oct 17, 2023

Wowowow I never thought I'd see the day! Nice work!

@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

40.0% 40.0% Coverage
0.0% 0.0% Duplication

@filipelautert
Copy link
Collaborator

Nice work!! Question for you - is it possible to fix this in the testcontainers repo, rather than fix it here? I'm sure there are other people who would like to get these fixes.

Good question! I think I can open an issue for them with my findings. It's quite simple - the code below makes it fail:

        Db2Container container = new Db2Container()
                .withUsername("lbuser")
                .withPassword("lbpass")
                .withDatabaseName("lbcat")
                .acceptLicense();

        List<PortBinding> portBindings = Collections.singletonList(new PortBinding(Ports.Binding.bindPort(50000), new ExposedPort(50000)));
        container.withCreateContainerCmdModifier(cmd -> cmd.withHostConfig(new HostConfig().withPortBindings(portBindings)));
        container.start();

If I exclude the container.withCreateContainerCmdModifier it works. We do it as part of our start method, but seems it is not required (at least for DB2) - and when it's done for DB2 it breaks the database startup. Also after finding it out I changed the PR again, now it's a lot more simple.

I'll create an issue at their github.

@filipelautert
Copy link
Collaborator

@@ -58,7 +59,7 @@ public void start() throws Exception {

container.withReuse(testSystem.getKeepRunning());

if (testSystem.getKeepRunning()) {
if (testSystem.getKeepRunning() && !(testSystem instanceof DB2TestSystem)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@abrackx in fact the entire fix is here.

@suryaaki2 suryaaki2 merged commit 22dd948 into master Oct 20, 2023
53 of 55 checks passed
@suryaaki2 suryaaki2 deleted the github-action-DAT-13113 branch October 20, 2023 13:47
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.

None yet

5 participants