Skip to content

Hackathon integration tests for checkout Test environment - #2037

Open
poojah-adyen wants to merge 1 commit into
mainfrom
hackathon-integrtaion-test-checkout
Open

Hackathon integration tests for checkout Test environment#2037
poojah-adyen wants to merge 1 commit into
mainfrom
hackathon-integrtaion-test-checkout

Conversation

@poojah-adyen

Copy link
Copy Markdown
Contributor

Description

Tested scenarios

Fixed issue:

@poojah-adyen
poojah-adyen requested a review from a team as a code owner August 14, 2026 11:45
@poojah-adyen poojah-adyen added the Don't Merge Don't merge yet for whatever reason label Aug 14, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a structured integration testing framework using Maven Failsafe, adding comprehensive documentation, configuration templates, and concrete integration tests for Checkout and Cloud Device APIs. Key feedback includes caching and properly closing the Client instance in BaseIntegrationTest to prevent resource leaks, safely loading classpath resources, supporting Java System properties for configuration overrides, and removing accidental system prompt tags from the documentation.

Comment on lines +24 to +34
protected final Client getClient() {
Environment environment = getEnvironment();
Config config = new Config().apiKey(getApiKey()).environment(environment);

String liveEndpointUrlPrefix = getOptionalProperty(LIVE_ENDPOINT_URL_PREFIX_PROPERTY);
if (environment == Environment.LIVE && liveEndpointUrlPrefix != null) {
config.liveEndpointUrlPrefix(liveEndpointUrlPrefix);
}

return new Client(config);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The getClient() method currently instantiates a new Client on every call. Since Client holds an underlying HTTP client with connection pools and threads, creating multiple unclosed instances can lead to resource leaks (such as socket or thread exhaustion) and slower test execution due to repeated TLS handshakes. Caching the client instance and closing it after each test using @AfterEach resolves these issues and improves performance through connection reuse.

  private Client client;

  protected final Client getClient() {
    if (client == null) {
      Environment environment = getEnvironment();
      Config config = new Config().apiKey(getApiKey()).environment(environment);

      String liveEndpointUrlPrefix = getOptionalProperty(LIVE_ENDPOINT_URL_PREFIX_PROPERTY);
      if (environment == Environment.LIVE && liveEndpointUrlPrefix != null) {
        config.liveEndpointUrlPrefix(liveEndpointUrlPrefix);
      }

      client = new Client(config);
    }
    return client;
  }

  @AfterEach
  protected void tearDownClient() throws IOException {
    if (client != null) {
      client.close();
      client = null;
    }
  }

Comment on lines +10 to +14
import com.adyen.enums.Environment;
import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;
import java.util.Properties;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Add the org.junit.jupiter.api.AfterEach import to support tearing down and closing the cached Client instance after each test execution.

Suggested change
import com.adyen.enums.Environment;
import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;
import java.util.Properties;
import com.adyen.enums.Environment;
import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;
import java.util.Properties;
import org.junit.jupiter.api.AfterEach;

Comment on lines +75 to +76
try (InputStream inputStream =
BaseIntegrationTest.class.getClassLoader().getResourceAsStream(CONFIGURATION_RESOURCE)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using BaseIntegrationTest.class.getClassLoader() can potentially return null if the class is loaded by the bootstrap classloader. It is safer and more idiomatic to use BaseIntegrationTest.class.getResourceAsStream with a leading slash to load classpath resources defensively.

Suggested change
try (InputStream inputStream =
BaseIntegrationTest.class.getClassLoader().getResourceAsStream(CONFIGURATION_RESOURCE)) {
try (InputStream inputStream =
BaseIntegrationTest.class.getResourceAsStream("/" + CONFIGURATION_RESOURCE)) {

Comment on lines +98 to +104
private static String getOptionalProperty(String name) {
String property = System.getenv(name);
if (property == null || property.isBlank()) {
property = PROPERTIES.getProperty(name);
}
return property == null || property.isBlank() ? null : property;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The configuration lookup currently only checks environment variables and the properties file. It does not check Java System properties (System.getProperty(name)). Adding System properties to the lookup chain allows developers to easily override configuration values via standard Maven command-line flags (e.g., -DADYEN_API_KEY=...).

  private static String getOptionalProperty(String name) {
    String property = System.getenv(name);
    if (property == null || property.isBlank()) {
      property = System.getProperty(name);
    }
    if (property == null || property.isBlank()) {
      property = PROPERTIES.getProperty(name);
    }
    return property == null || property.isBlank() ? null : property;
  }

@@ -0,0 +1,59 @@
<integration_test_guidelines>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The XML-like tags <integration_test_guidelines> at the beginning of the file and </integration_test_guidelines> at the end of the file appear to be system prompt artifacts that were accidentally committed. They should be removed to keep the markdown clean and professional.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Don't Merge Don't merge yet for whatever reason

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant