Skip to content

CLDSRV-835: Fix flaky cleanup and teardown s3C related tests #6109

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.

Sign up for GitHub

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
bert-e merged 4 commits into from
Mar 18, 2026
Merged

CLDSRV-835: Fix flaky cleanup and teardown s3C related tests #6109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
619bdeb
CLDSRV-835: Fix flaky async await provideRawOutput
BourgoisMickael Mar 13, 2026
9471b0c
CLDSRV-835: Normalize async provideRawOutput
BourgoisMickael Mar 13, 2026
12097a6
CLDSRV-835: Isolate bucket name for put object
BourgoisMickael Mar 13, 2026
3a5f77a
CLDSRV-835: Improve bucket utility empty function
BourgoisMickael Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 54 additions & 36 deletions tests/functional/aws-node-sdk/lib/utility/bucket-util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const {
CreateBucketCommand,
DeleteBucketCommand,
ListObjectVersionsCommand,
DeleteObjectsCommand,
DeleteObjectCommand,
ListBucketsCommand,
} = require('@aws-sdk/client-s3');
Expand Down Expand Up @@ -90,48 +91,65 @@ class BucketUtility {
* @param bucketName
* @returns {Promise.<T>}
*/
empty(bucketName, BypassGovernanceRetention = false) {
const param = {
Bucket: bucketName,
};

return this.s3.send(new ListObjectVersionsCommand(param))
.then(data => Promise.all(
(data.Versions || [])
.filter(object => !object.Key.endsWith('/'))
.map(object =>
this.s3.send(new DeleteObjectCommand({
async empty(bucketName, BypassGovernanceRetention = false) {
let keyMarker = undefined;
let versionIdMarker = undefined;
let isTruncated = true;

while (isTruncated) {
const response = await this.s3.send(new ListObjectVersionsCommand({
Bucket: bucketName,
KeyMarker: keyMarker,
VersionIdMarker: versionIdMarker,
}));

const objects = [
...(response.Versions || []),
...(response.DeleteMarkers || []),
].map(({ Key, VersionId }) => ({ Key, VersionId }));

if (objects.length > 0) {
try {
const result = await this.s3.send(new DeleteObjectsCommand({
Bucket: bucketName,
Key: object.Key,
VersionId: object.VersionId,
Delete: {
Objects: objects,
Quiet: true
},
...(BypassGovernanceRetention && { BypassGovernanceRetention }),
})).then(() => object)
)
.concat((data.Versions || [])
.filter(object => object.Key.endsWith('/'))
.map(object =>
}));
if (result.Errors && result.Errors.length > 0) {
for (const e of result.Errors) {
// eslint-disable-next-line no-console
console.warn(
`Warning BucketUtility.empty(): failed to delete s3://${bucketName}/${e.Key}` +
` (versionId=${e.VersionId}): ${e.Code} - ${e.Message}`
);
}
}
} catch (err) {
// Older cloudserver versions reject DeleteObjects with BadDigest
// due to a Content-MD5 integrity check issue. Fall back to individual deletes.
if (err.name !== 'BadDigest') {
throw err;
}
await Promise.all(objects.map(({ Key, VersionId }) =>
this.s3.send(new DeleteObjectCommand({
Bucket: bucketName,
Key: object.Key,
VersionId: object.VersionId,
Key,
VersionId,
...(BypassGovernanceRetention && { BypassGovernanceRetention }),
}))
.then(() => object)
)
)
.concat((data.DeleteMarkers || [])
.map(object =>
this.s3.send(new DeleteObjectCommand({
Bucket: bucketName,
Key: object.Key,
VersionId: object.VersionId,
...(BypassGovernanceRetention && { BypassGovernanceRetention }),
}))
.then(() => object)
)
)
)
);
));
}
}

isTruncated = response.IsTruncated;
if (isTruncated) {
keyMarker = response.NextKeyMarker;
versionIdMarker = response.NextVersionIdMarker;
}
}
}

emptyMany(bucketNames) {
Expand Down
6 changes: 6 additions & 0 deletions tests/functional/aws-node-sdk/lib/utility/provideRawOutput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const cp = require('child_process');
const util = require('util');

const conf = require('../../../../../lib/Config').config;

Expand Down Expand Up @@ -44,4 +45,9 @@ function provideRawOutput(args, cb) {
});
}

provideRawOutput[util.promisify.custom] = args =>
new Promise(resolve =>
provideRawOutput(args, (httpCode, rawOutput) => resolve({ httpCode, rawOutput }))
);
Copy link
Copy Markdown

@claude claude bot Mar 16, 2026

Choose a reason for hiding this comment

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

The provideRawOutput callback has a non-standard signature (httpCode, rawOutput) — no err first argument. The custom promisify implementation correctly handles this by always resolving (never rejecting). However, if curl crashes or the child process errors, the promise resolves with httpCode as undefined rather than rejecting. Consider adding a rejection path for unexpected failures, or at minimum a short comment explaining the always-resolves design choice.

— Claude Code

Copy link
Copy Markdown
Contributor Author

@BourgoisMickael BourgoisMickael Mar 16, 2026

Choose a reason for hiding this comment

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

It was like that before, it will be the assert on httpCode that will fail


module.exports = provideRawOutput;
74 changes: 22 additions & 52 deletions tests/functional/aws-node-sdk/test/legacy/authV2QueryTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const assert = require('assert');
const process = require('node:process');
const cp = require('child_process');
const util = require('util');
const timers = require('timers/promises');
const {
S3Client,
CreateBucketCommand,
Expand All @@ -12,6 +14,7 @@ const {
const { getSignedUrl } = require('@aws-sdk/s3-request-presigner');
const getConfig = require('../support/config');
const provideRawOutput = require('../../lib/utility/provideRawOutput');
const provideRawOutputAsync = util.promisify(provideRawOutput);

const random = Math.round(Math.random() * 100).toString();
const bucket = `mybucket-${random}`;
Expand Down Expand Up @@ -60,49 +63,32 @@ describe('aws-node-sdk v2auth query tests', function testSuite() {
const futureExpiry = Math.floor(Date.now() / 1000) + 604810; // 10 seconds more than limit
urlObj.searchParams.set('Expires', futureExpiry.toString());
const invalidUrl = urlObj.toString();
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'PUT', invalidUrl], httpCode => {
assert.strictEqual(httpCode, '403 FORBIDDEN');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'PUT', invalidUrl]);
assert.strictEqual(httpCode, '403 FORBIDDEN');
});

it('should return an error code if request occurs after expiry', async () => {
const command = new CreateBucketCommand({ Bucket: bucket });
const url = await getSignedUrl(s3, command, { expiresIn: 1 });
await new Promise(resolve => {
setTimeout(() => {
provideRawOutput(['-verbose', '-X', 'PUT', url], httpCode => {
assert.strictEqual(httpCode, '403 FORBIDDEN');
resolve();
});
}, 1500);
});
await timers.setTimeout(1500);
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'PUT', url]);
assert.strictEqual(httpCode, '403 FORBIDDEN');
});

it('should create a bucket', async () => {
const command = new CreateBucketCommand({ Bucket: bucket });
const url = await getSignedUrl(s3, command, { expiresIn: almostOutsideTime });
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'PUT', url], httpCode => {
assert.strictEqual(httpCode, '200 OK');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'PUT', url]);
assert.strictEqual(httpCode, '200 OK');
});


it('should put an object', async () => {
const command = new PutObjectCommand({ Bucket: bucket, Key: 'key' });
const url = await getSignedUrl(s3, command, { expiresIn: almostOutsideTime });
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'PUT', url,
'--upload-file', 'uploadFile'], httpCode => {
assert.strictEqual(httpCode, '200 OK');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'PUT', url,
'--upload-file', 'uploadFile']);
assert.strictEqual(httpCode, '200 OK');
});

it('should put an object with an acl setting and a storage class setting', async () => {
Expand All @@ -118,25 +104,17 @@ describe('aws-node-sdk v2auth query tests', function testSuite() {
StorageClass: 'STANDARD'
});
const url = await getSignedUrl(s3, command);
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'PUT', url,
'--upload-file', 'uploadFile'], httpCode => {
assert.strictEqual(httpCode, '200 OK');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'PUT', url,
'--upload-file', 'uploadFile']);
assert.strictEqual(httpCode, '200 OK');
});


it('should get an object', async () => {
const command = new GetObjectCommand({ Bucket: bucket, Key: 'key' });
const url = await getSignedUrl(s3, command, { expiresIn: almostOutsideTime });
await new Promise(resolve => {
provideRawOutput(['-verbose', '-o', 'download', url], httpCode => {
assert.strictEqual(httpCode, '200 OK');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-o', 'download', url]);
assert.strictEqual(httpCode, '200 OK');
});

it('downloaded file should equal file that was put', done => {
Expand All @@ -148,23 +126,15 @@ describe('aws-node-sdk v2auth query tests', function testSuite() {
it('should delete an object', async () => {
const command = new DeleteObjectCommand({ Bucket: bucket, Key: 'key' });
const url = await getSignedUrl(s3, command, { expiresIn: almostOutsideTime });
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'DELETE', url], httpCode => {
assert.strictEqual(httpCode, '204 NO CONTENT');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'DELETE', url]);
assert.strictEqual(httpCode, '204 NO CONTENT');
});


it('should delete a bucket', async () => {
const command = new DeleteBucketCommand({ Bucket: bucket });
const url = await getSignedUrl(s3, command, { expiresIn: almostOutsideTime });
await new Promise(resolve => {
provideRawOutput(['-verbose', '-X', 'DELETE', url], httpCode => {
assert.strictEqual(httpCode, '204 NO CONTENT');
resolve();
});
});
const { httpCode } = await provideRawOutputAsync(['-verbose', '-X', 'DELETE', url]);
assert.strictEqual(httpCode, '204 NO CONTENT');
});
});
Loading
Loading