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

Improving Test Reliability in RemoteTemporaryDirectory and TestJobManager #452

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

herve-er
Copy link
Contributor

@herve-er herve-er commented Jan 3, 2025

Issue 1: Naming Conflicts in RemoteTemporaryDirectory

RemoteTemporaryDirectory can encounter a FileAlreadyExist error due to naming conflicts in the following scenarios:

  • Simultaneous test execution by two runners running the same test.
  • Unnamed tests started within a 1-minute interval.

Solution:

  • A random suffix is added to the directory name to significantly reduce the likelihood of collisions.
  • In the rare case where a collision still occurs (e.g., simultaneous execution and matching random suffix), the error will be caught, and the system will retry with a new random suffix.

Issue 2: Flaky Test in TestJobManager::testCancelJobs

The TestJobManager::testCancelJobs test could fail randomly due to:

  • Dependency on whether 100 upload jobs completed in less than 1 second.

Solution:

  • Transition from relying on a predetermined wait time to a more robust approach:
    1. Wait for at least one job to complete.
    2. Immediately cancel all jobs.
    3. Wait for the jobManager queue to empty.
    4. Perform the existing checks.

This change not only improves test reliability but also reduces test execution time, eliminating the need for an arbitrary 11-second wait.


Issue 3: Flaky Test in testLog::testExpiredLogFiles

testLog::testExpiredLogFiles could fail randomly due to timing issues caused by too short wait for log expiration.

Solution:

Increased the time interval for log expiration to ensure adequate margin for the test environment. This change ensures stability and eliminates random failures.

@herve-er herve-er added this to the 3.6.8 milestone Jan 3, 2025
@herve-er herve-er self-assigned this Jan 3, 2025
@herve-er herve-er requested a review from a team as a code owner January 3, 2025 10:57
@herve-er herve-er marked this pull request as draft January 3, 2025 12:30
@herve-er herve-er changed the title Prevent name collision in RemoteTemporaryDirectory Improving Test Reliability in RemoteTemporaryDirectory and TestJobManager Jan 3, 2025
@herve-er herve-er marked this pull request as ready for review January 3, 2025 14:34
@herve-er herve-er enabled auto-merge (squash) January 3, 2025 14:49
Copy link

sonarqubecloud bot commented Jan 3, 2025


cancelAllOngoingJobs();

Utility::msleep(10000); // Wait 10sec
int rerty = 1000; // Wait max 10sec
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

size_t total = data->size();
CPPUNIT_ASSERT(jobCounter != total);
CPPUNIT_ASSERT(total > 0);
size_t uploadedFileCounter = data->size();
Copy link
Contributor

Choose a reason for hiding this comment

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

const

_dirId = dataObj->get(idKey).toString();
const std::string& testType /*= "undef"*/) :
_driveDbId(driveDbId) {
std::string suffix = CommonUtility::generateRandomStringAlphaNum(5);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not an issue but you can put this into the loop so you won't need to call it again on line 46.

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