Improvement/cldsrv 724-backbeat-related-functional-tests #5985

benzekrimaha · 2025-10-23T16:14:03Z

Please note that this PR also takles the rest of the unmigrated code , better to review it by commit to have the context

Issue: CLDSRV-724

codecov · 2025-11-05T08:18:31Z

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.35%. Comparing base (0fec989) to head (df480a2).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
lib/routes/routeBackbeat.js	33.33%	6 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/routes/routeBackbeat.js	`75.79% <33.33%> (+2.40%)`	⬆️

... and 28 files with indirect coverage changes

@@                                       Coverage Diff                                       @@
##           improvement/CLDSRV-724-service-get-related-functional-tests    #5985      +/-   ##
===============================================================================================
+ Coverage                                                        81.00%   84.35%   +3.34%     
===============================================================================================
  Files                                                              204      204              
  Lines                                                            12890    12899       +9     
===============================================================================================
+ Hits                                                             10442    10881     +439     
+ Misses                                                            2448     2018     -430

Flag	Coverage Δ
file-ft-tests	`67.55% <0.00%> (+7.42%)`	⬆️
kmip-ft-tests	`28.11% <0.00%> (+<0.01%)`	⬆️
mongo-v0-ft-tests	`68.78% <0.00%> (+7.30%)`	⬆️
mongo-v1-ft-tests	`68.88% <0.00%> (+7.42%)`	⬆️
multiple-backend	`35.23% <0.00%> (-0.03%)`	⬇️
sur-tests	`36.35% <0.00%> (+0.78%)`	⬆️
sur-tests-inflights	`37.39% <0.00%> (-0.03%)`	⬇️
unit	`69.90% <33.33%> (-0.03%)`	⬇️
utapi-v2-tests	`34.22% <0.00%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tests/functional/raw-node/test/GCP/bucket/get.js

SylvainSenechal · 2025-12-09T15:26:34Z

tests/functional/sse-kms-migration/load.js

 * round robin is confined to each nodejs cluster processes
 */
-const APPROX = Math.floor(0.1 * TOTAL_OBJECTS_PER_NODE);
+const APPROX = Math.ceil(0.2 * TOTAL_OBJECTS_PER_NODE);


do you remember what happened here that you need to do this change ?

The old Math.floor(0.1 * TOTAL_OBJECTS_PER_NODE) let counts drift only ~10% low/high; on the ci we were still failing because a cluster process often handles more than 10%. Switching to Math.ceil(0.2 * TOTAL_OBJECTS_PER_NODE) widens that window to ±20% (and guarantees at least one packet of slack), so we stop treating normal round‑robin skew as a test failure.

SylvainSenechal

bigbig pr again. this time I left a bit more comments but actually not many changes requested, its mostly questions to double check the changes, and make sure we understand why the changes are done because I found 2/3 things were curious

DarkIsDude

Congrats again. Sylvain did a good review, not a lot to add more

DarkIsDude · 2025-12-12T16:14:13Z

.github/workflows/tests.yaml

-        password: ${{ secrets.ARTIFACTS_PASSWORD }}
-        source: /tmp/artifacts
-      if: always()
+  # ceph-backend-test:


Just delete them ? We have githistory if needed

DarkIsDude · 2025-12-12T16:22:20Z

lib/data/wrapper.js

    client = new DataFileInterface(config);
    implName = 'file';
 } else if (config.backends.data === 'multiple') {
+    Object.keys(config.locationConstraints).filter(k => 


Agree too, it's weird ?

DarkIsDude · 2025-12-12T16:31:18Z

tests/functional/aws-node-sdk/test/bucket/getLocation.js

-                const expectedLocation = data.LocationConstraint || '';
-                assert.deepStrictEqual(expectedLocation, '');
+                // SDK v3 returns undefined for us-east-1, normalize to empty string for comparison
+                const locationConstraint = data.LocationConstraint || '';


Then if I get it right we can check that the value is undefined directly ?

DarkIsDude · 2025-12-12T16:36:14Z

tests/functional/aws-node-sdk/test/mdSearch/utils/helpers.js

+                    done();
                }
+            } else if (testResult && typeof testResult === 'object' && testResult.code) {
+                // This was expected to be an error, but we got success


Suggested change

// This was expected to be an error, but we got success

tests/functional/aws-node-sdk/test/multipleBackend/acl/aclAwsVersioning.js

DarkIsDude · 2025-12-12T16:39:27Z

tests/functional/aws-node-sdk/test/multipleBackend/delete/deleteAwsVersioning.js

        'backend successfully', done => {
            const key = `somekey-${genUniqID()}`;
            async.waterfall([
+                next => waitForVersioningBeforePut(s3, bucket, err => {


Same here and following iteration

DarkIsDude · 2025-12-12T17:11:03Z

tests/functional/aws-node-sdk/test/multipleBackend/objectPutCopyPart/objectPutCopyPartAzure.js

+                    next => {
+                            s3.send(new AbortMultipartUploadCommand(paramsAzure))
+                                    .then(() => next())
+                                    .catch(next);
+                    },


Suggested change

next => {

s3.send(new AbortMultipartUploadCommand(paramsAzure))

.then(() => next())

.catch(next);

},

async() => s3.send(new AbortMultipartUploadCommand(paramsAzure)),

we can do that ?

tests/functional/aws-node-sdk/test/multipleBackend/put/putGcp.js

DarkIsDude · 2025-12-12T17:32:17Z

tests/functional/backbeat/excludedDataStoreName.js

        });
    });
 });
+


Suggested change

DarkIsDude · 2025-12-12T17:34:36Z

tests/functional/backbeat/listLifecycleCurrents.js

+            }))
+                .then(() => next())
+                .catch(next),
+            next => s3.send(new DeleteObjectCommand({ Bucket: testBucket, Key: keyName1 }))


Here too (and it's not the only one)

Suggested change

next => s3.send(new DeleteObjectCommand({ Bucket: testBucket, Key: keyName1 }))

async () => s3.send(new DeleteObjectCommand({ Bucket: testBucket, Key: keyName1 })),

I'd rather keep the next here it's part of a chain

DarkIsDude

Most of them are nit. That's why it's approved. You can resolve them and merge or reassign me.

DarkIsDude · 2026-01-06T15:09:38Z

package.json

    "@hapi/joi": "^17.1.1",
    "@smithy/node-http-handler": "^3.0.0",
-    "arsenal": "git+https://github.com/scality/Arsenal#8.2.41",
+    "arsenal": "git+https://github.com/scality/Arsenal#d1de8c4ac819ee7363323fa8ab14fcdba37a5e65",


bump arsenal, reminder for you

DarkIsDude · 2026-01-06T15:15:20Z

tests/functional/aws-node-sdk/test/bucket/getLocation.js

-                const expectedLocation = data.LocationConstraint || '';
-                assert.deepStrictEqual(expectedLocation, '');
+                // SDK v3 returns undefined for us-east-1, normalize to empty string for comparison
+                const locationConstraint = data.LocationConstraint || '';


DarkIsDude · 2026-01-06T15:15:31Z

tests/functional/aws-node-sdk/test/bucket/getLocation.js

+            after(() => {
+                process.stdout.write('Deleting bucket\n');
+                return bucketUtil.deleteOne(bucketName)
+                .catch(err => {


Indent 🙏

tests/functional/aws-node-sdk/test/bucket/getLocation.js

DarkIsDude · 2026-01-06T15:17:35Z

tests/functional/aws-node-sdk/test/bucket/getLocation.js

                const data = await s3.send(new GetBucketLocationCommand({ Bucket: bucketName }));
-                assert.strictEqual(data.LocationConstraint, endpoint);
+
+                // S3C backend has 'dc-1' as default location constraint


I think the code is pretty clean alone ?

DarkIsDude · 2026-01-06T15:40:46Z

tests/functional/aws-node-sdk/test/multipleBackend/utils.js

-utils.enableVersioning = async (s3, bucket) => {
-    await s3.send(new PutBucketVersioningCommand({ 
+utils.enableVersioning = (s3, bucket, callback) => {
+    // Support both callback and promise patterns


Suggested change

// Support both callback and promise patterns

That's not needed

DarkIsDude · 2026-01-06T15:41:03Z

tests/functional/aws-node-sdk/test/multipleBackend/utils.js

+    try {
+        const results = [];
+        for (const data of dataArray) {
+            // Don't pass callback to putToAwsBackend - we need the version ID from the promise


Suggested change

// Don't pass callback to putToAwsBackend - we need the version ID from the promise

DarkIsDude · 2026-01-06T15:48:04Z

tests/functional/aws-node-sdk/test/support/awsConfig.js


-    return fromIni({ profile, filepath: filename });
+    // Parse the INI file manually for synchronous access
+    const content = fs.readFileSync(filename, 'utf-8');


Ouch, do we really need that ? Why this change and not using the SDK or access the SDK config ?

DarkIsDude · 2026-01-06T16:07:43Z

tests/functional/raw-node/test/GCP/object/completeMpu.js

                MultipartUpload: { Parts: parts },
            };
            gcpClient.completeMultipartUpload(params, (err, res) => {
+                // eslint-disable-next-line no-console


should be removed or the linter rule should be updated 🙏 ?

DarkIsDude · 2026-01-06T16:07:58Z

tests/functional/raw-node/test/GCP/object/completeMpu.js


        it('should successfully complete MPU',
        function testFn(done) {
+            this.timeout(1200000);


Maybe explain the why here ?

removed, ci passing without it

SylvainSenechal

last comments addressed in another pr

benzekrimaha force-pushed the improvement/CLDSRV-724-service-get-related-functional-tests branch 3 times, most recently from 72dd6ed to cd35cca Compare October 28, 2025 15:59

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch 2 times, most recently from 425731c to 2b9959d Compare October 28, 2025 16:03

scality deleted a comment from codecov bot Nov 5, 2025

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch 5 times, most recently from 8c9eb10 to 3ab4eae Compare November 13, 2025 10:44

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch 2 times, most recently from 17d2fcb to 198582c Compare November 20, 2025 17:19

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch 3 times, most recently from 2c806e4 to 836c556 Compare November 28, 2025 15:18

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch 2 times, most recently from 26ce909 to 8383cf9 Compare December 2, 2025 17:24

benzekrimaha force-pushed the improvement/CLDSRV-724-service-get-related-functional-tests branch from cd35cca to 0fec989 Compare December 4, 2025 06:39

benzekrimaha changed the title ~~Improvement/cldsrv 724 backbeat related functional tests~~ Improvement/cldsrv 724-backbeat-related-functional-tests Dec 4, 2025

benzekrimaha added 9 commits December 4, 2025 12:35

backbeat functional tests migration

a1fff68

aws-node-sdk bucket functional tests migration

91fe932

aws-node-sdk multiple backend tests migration

7408990

raw-node gcp object and bucket tests migration

0ff38c1

routBackbeat migration adaptation

1c371b1

aws-node-sdk test utils migration

03dc295

raw-node rest of code migration

ac83417

rest of code migration

b07e438

deps install

c2f4c3a

SylvainSenechal reviewed Dec 9, 2025

View reviewed changes

tests/functional/raw-node/test/GCP/bucket/get.js Outdated Show resolved Hide resolved

SylvainSenechal reviewed Dec 9, 2025

View reviewed changes

SylvainSenechal requested changes Dec 9, 2025

View reviewed changes

fixups post reviews

5313153

benzekrimaha requested review from a team, SylvainSenechal and delthas December 11, 2025 18:40

DarkIsDude approved these changes Dec 12, 2025

View reviewed changes

DarkIsDude requested changes Dec 12, 2025

View reviewed changes

benzekrimaha added 3 commits December 18, 2025 14:12

bump arsenal with latest fixups

eee9699

bump arsenal with latest fixups

1d9770c

bump arsenal with latest fixups

7e3a81d

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch from 3a0af19 to 7e3a81d Compare December 18, 2025 16:30

benzekrimaha added 3 commits December 18, 2025 18:17

bump arsenal with latest fixups

4f3c944

bump arsenal with latest fixups

f9fbace

bump arsenal with latest fixups

ce181cb

benzekrimaha force-pushed the improvement/CLDSRV-724-backbeat-related-functional-tests branch from 94d9c72 to ce181cb Compare December 19, 2025 06:24

benzekrimaha added 10 commits December 19, 2025 07:52

bump arsenal with latest fixups

42ed39e

bump arsenal with latest fixups

908ba8d

bump arsenal with latest fixups

e30a7b0

bump arsenal with latest fixups

c73a9c1

bump arsenal with latest fixups

31d137c

fixups post reviews

80e3e93

fixups post reviews

82472d0

fixups post reviews

74d7db3

fixups post reviews

1783b09

fixups post reviews

df480a2

benzekrimaha requested a review from DarkIsDude December 22, 2025 17:22

DarkIsDude approved these changes Jan 6, 2026

View reviewed changes

SylvainSenechal approved these changes Jan 7, 2026

View reviewed changes

	next => s3.send(new DeleteObjectCommand({ Bucket: testBucket, Key: keyName1 }))
	async () => s3.send(new DeleteObjectCommand({ Bucket: testBucket, Key: keyName1 })),

Improvement/cldsrv 724-backbeat-related-functional-tests #5985

Are you sure you want to change the base?

Improvement/cldsrv 724-backbeat-related-functional-tests #5985

Conversation

benzekrimaha commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal left a comment

Choose a reason for hiding this comment

Uh oh!

DarkIsDude left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DarkIsDude left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benzekrimaha commented Oct 23, 2025 •

edited

Loading

codecov bot commented Nov 5, 2025 •

edited

Loading