Skip to content

Fix intermittent failures in AccountServiceTests #78

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

Merged
merged 3 commits into from
May 20, 2025

Conversation

andreatamezm
Copy link
Contributor

@andreatamezm andreatamezm commented May 20, 2025

Problem
The testPaginatedAccountList test in AccountServiceTests was experiencing intermittent 500 errors when searching for newly created accounts:
GenabilityException: Failed GET http://family-main-preview/rest/v1/accounts?fields=ext&pageCount=5&search=JAVA+CLIENT+TEST+ACCOUNT&searchOn=accountName: HTTP error code : 500
The issue occurred because:

  1. Multiple test runs would create accounts with identical names
  2. The server's search index needed time to update after account creation
  3. No resilience against temporary server errors when multiple tests ran concurrently

Previous PR: #76 (tackling each tests separately now)

@andreatamezm andreatamezm requested a review from a team May 20, 2025 16:32
try {
restResponse = accountService.getAccounts(request);
break; // Success, exit the retry loop
} catch (GenabilityException e) {
Copy link

Choose a reason for hiding this comment

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

I am assuming the not found case / 404 is a Genability Exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the not found case is a 500

GenabilityException: Failed GET http://family-main-preview/rest/v1/accounts?fields=ext&pageCount=5&search=JAVA+CLIENT+TEST+ACCOUNT&searchOn=accountName: HTTP error code : 500

throw e; // Re-throw if we've exhausted retries
}

System.out.println("Search request failed, retrying (" + retryCount + "/" + maxRetries + ")");
Copy link

@mihle mihle May 20, 2025

Choose a reason for hiding this comment

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

I would remove this logging line. If we need to log, we should use a framework provided logging.

@andreatamezm andreatamezm requested a review from mihle May 20, 2025 16:46
Copy link

@mihle mihle left a comment

Choose a reason for hiding this comment

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

I would also consider breaking out the magic numbers (maxretry, sleep time) as statics on top of the class.

@andreatamezm andreatamezm merged commit 949ac93 into main May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants