Improvement/cldsrv 561 add tests for create and store object by benzekrimaha · Pull Request #6082 · scality/cloudserver

benzekrimaha · 2026-02-23T09:34:22Z

This PR strengthens coverage around createAndStoreObject, the core path used by putObject, putRestoredObject, and putObjectVersion, with emphasis on cold-storage restore and archived-object edge cases.
The goal is to improve confidence in behavior-heavy flows
Issue: CLDSRV-561

bert-e · 2026-02-23T09:34:25Z

Hello benzekrimaha,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-02-23T09:34:30Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-561 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.3.4

Please check the Fix Version/s of CLDSRV-561, or the target
branch of this pull request.

codecov · 2026-02-23T09:41:00Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.21%. Comparing base (2a7a68f) to head (21a9df6).
⚠️ Report is 60 commits behind head on development/9.3.
✅ All tests successful. No failed tests found.

Additional details and impacted files

see 4 files with indirect coverage changes

@@                 Coverage Diff                 @@
##           development/9.3    #6082      +/-   ##
===================================================
- Coverage            84.31%   84.21%   -0.10%     
===================================================
  Files                  206      206              
  Lines                13251    13251              
===================================================
- Hits                 11172    11159      -13     
- Misses                2079     2092      +13

Flag	Coverage Δ
file-ft-tests	`63.59% <ø> (-4.33%)`	⬇️
kmip-ft-tests	`28.08% <ø> (-0.04%)`	⬇️
mongo-v0-ft-tests	`69.14% <ø> (+<0.01%)`	⬆️
mongo-v1-ft-tests	`69.25% <ø> (+0.03%)`	⬆️
multiple-backend	`35.10% <ø> (ø)`
sur-tests	`36.46% <ø> (+0.79%)`	⬆️
sur-tests-inflights	`37.49% <ø> (+<0.01%)`	⬆️
unit	`69.90% <ø> (ø)`
utapi-v2-tests	`34.36% <ø> (ø)`

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.

bert-e · 2026-02-23T09:41:11Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

bert-e · 2026-02-24T08:52:09Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-561 contains:

9.3.4

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.3.5

Please check the Fix Version/s of CLDSRV-561, or the target
branch of this pull request.

Issue: CLDSRV-561

bert-e · 2026-02-24T09:05:25Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-561 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.3.5

Please check the Fix Version/s of CLDSRV-561, or the target
branch of this pull request.

Issue: CLDSRV-561

bert-e · 2026-02-24T11:12:11Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

francoisferrand

Looking at code coverage (https://app.codecov.io/github/scality/cloudserver/pull/6082/blob/lib/api/apiUtils/object/createAndStoreObject.js?dropdown=coverage), we can confirm createAndStoreObject is already reletively well "covered".

There are some gaps indeed, but this PR does not cover them.

However, the missing parts is also sometimes more subtle, and requires checking the actual behavior : like do we set the right arsenal flag (needOplogUpdate)...

So it is not just adding tests:

need to dig deeper to identify the specific behaviors which must be implemented by this function (like when to require oplog update, the originOp, etc...)
then either find the tests which already cover these cases (there are some, c.f. coverage) and update them; or indeed add some extra tests to validate these specific behaviors

tests/functional/aws-node-sdk/test/object/objectOverwrite.js

bert-e · 2026-02-25T16:39:02Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-561 contains:

9.3.5

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.3.5
9.4.0

Please check the Fix Version/s of CLDSRV-561, or the target
branch of this pull request.

bert-e · 2026-02-27T15:39:36Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

SylvainSenechal

wouldn't we want (some of) these tests to be in zenko, to avoid relying on mocks ? I guess should be possible, running mongo commands to directly inspect database data

francoisferrand · 2026-03-11T08:43:29Z

wouldn't we want (some of) these tests to be in zenko, to avoid relying on mocks ? I guess should be possible, running mongo commands to directly inspect database data

some (if not all) of these tests are already in Zenko, as part of functional / e2e tests : i.e. can we restore an object (in the various cases)? do we get the proper bucket notification? ... But IMHO "too late" (in the dev process) to inspect mongo content in Zenko tests.

relying on mocks is good to test corner cases, and ideally we would have integration tests to cross-check the actual backend works as expected: i.e. running some tests against mongo to verify it behaves as expected. How relevant (or not) the integration tests are really depends on the specific case: e.g. if we are already sure mongo (or metadata) write the object MD as we provide them, then good enough to verify we call this metadata layer with the right information....

But we should try to keep these tests as close to the code as possible: i.e. we want to make sure Cloudserver behaves as expected with mongo backend, should be done here (but integration between backbeat and cloudserver would probably be in Zenko)

francoisferrand · 2026-03-16T13:28:17Z

tests/functional/aws-node-sdk/test/object/objectOverwrite.js

+        [
+            {
+                name: 'transition in progress',
+                setup: () => fakeMetadataTransitionPromise(bucketName, objectName, undefined),


the difference between these "setup" is really just the last parameter of fakeMetadataArchivePromise : instead of putting a setup() function, it would be more readable to specify that parameter only (with a meaningful name) and keep the whole logic in the test body

francoisferrand · 2026-03-16T13:30:57Z

tests/functional/aws-node-sdk/test/object/objectOverwrite.js

+const fakeMetadataArchivePromise = promisify(fakeMetadataArchive);
+const fakeMetadataTransitionPromise = promisify(fakeMetadataTransition);
+const getMetadataPromise = promisify(getMetadata);
+const initMetadataPromise = promisify(initMetadata);


as per our "async migration" process, should we not take the chance to promisify these functions directly (not update every place they are used, but make them optionally async) ?

(not really the case we discussed -this is not the code we modify-, but we don't take the step we will never migrate such code...)

francoisferrand · 2026-03-16T13:34:57Z

tests/unit/api/createAndStoreObject.js

+                url: `/${bucketName}/${objectKey}`,
+            }, Buffer.from('test data', 'utf8'));
+
+            createAndStoreObject(bucketName, testBucket, objectKey, null,


should be promisified, to write "modern" async code?

Issue: CLDSRV-561

claude · 2026-03-25T08:50:47Z

tests/unit/api/createAndStoreObject.js

+            const storedObjMD = getStoredObjectData();
+            assert.strictEqual(storedObjMD.bucketOwnerId, canonicalID);
+        });
+        });


Extra }); here causes the outer describe('createAndStoreObject') block to close prematurely. All subsequent describe blocks (Delete marker creation, Archived object replacement, Cold storage restoration, etc.) end up outside the main describe, so the beforeEach/afterEach hooks (which set up testBucket, run cleanup(), and call sinon.restore()) will not execute for those tests.

Remove this extra line.

```suggestion

<br><br>— Claude Code

claude · 2026-03-25T08:50:54Z

tests/unit/api/createAndStoreObject.js:121 — Extra }); prematurely closes the outer describe('createAndStoreObject') block. All test suites after 'Regular object creation' lose access to beforeEach/afterEach hooks (testBucket setup, cleanup(), sinon.restore()), meaning those tests either fail or pass without proper isolation.

Review by Claude Code

claude · 2026-03-25T09:03:02Z

tests/functional/aws-node-sdk/test/object/putVersion.js

+                let putVersionId;
+                const fakeMetadataArchivePromise = promisify(fakeMetadataArchive);
+                const putObjectVersionPromise = promisify(putObjectVersion);
+                const getMetadataPromise = promisify(getMetadata);


promisify is unnecessary here — fakeMetadataArchive, putObjectVersion, and getMetadata already return promises when called without a callback (see the dual-mode refactoring in init.js in this same PR). You can call them directly and drop the promisify wrappers and the promisify import.

— Claude Code

claude · 2026-03-25T09:03:11Z

putVersion.js: promisify wrappers are redundant — the functions already return promises when called without a callback (dual-mode refactoring in this same PR's init.js changes).

Review by Claude Code

claude · 2026-03-25T15:18:58Z

tests/functional/aws-node-sdk/test/object/putVersion.js

-        next(err);
-    }
+        return res;
+    })();


Unnecessary IIFE — s3.send(command) already returns a promise, so the async wrapper adds nothing.

```suggestion
const promise = s3.send(command);

claude · 2026-03-25T15:19:05Z

Unnecessary async IIFE in putObjectVersion (putVersion.js:52-55): s3.send(command) already returns a promise, the (async () => { ... })() wrapper is redundant.

Review by Claude Code

benzekrimaha force-pushed the improvement/CLDSRV-561-add-tests-for-createAndStoreObject branch from c6e0798 to dddff1b Compare February 24, 2026 08:52

add unit tests for createAndStoreObject

d5fdd77

Issue: CLDSRV-561

benzekrimaha force-pushed the improvement/CLDSRV-561-add-tests-for-createAndStoreObject branch from dddff1b to 84844ff Compare February 24, 2026 08:53

benzekrimaha added 2 commits February 24, 2026 12:10

add functional restore via objectOverwite tests

1ec4d6f

Issue: CLDSRV-561

add ingestion-specific restore functional test

57227d6

Issue: CLDSRV-561

benzekrimaha force-pushed the improvement/CLDSRV-561-add-tests-for-createAndStoreObject branch from 0f0e6e9 to 57227d6 Compare February 24, 2026 11:10

benzekrimaha marked this pull request as ready for review February 24, 2026 11:11

benzekrimaha requested a review from francoisferrand February 24, 2026 11:12

benzekrimaha requested review from a team and SylvainSenechal February 24, 2026 11:12

francoisferrand reviewed Feb 24, 2026

View reviewed changes

benzekrimaha added 2 commits February 24, 2026 18:04

post review fixups

3d5ca31

tests adding

ac53023

benzekrimaha requested a review from francoisferrand February 27, 2026 15:39

SylvainSenechal approved these changes Feb 27, 2026

View reviewed changes

benzekrimaha requested review from a team and delthas March 4, 2026 09:48

scality deleted a comment from bert-e Mar 16, 2026

francoisferrand reviewed Mar 16, 2026

View reviewed changes

francoisferrand approved these changes Mar 16, 2026

View reviewed changes

delthas removed their request for review March 20, 2026 08:33

modernize metadata helpers and async tests

cebeb28

Issue: CLDSRV-561

claude bot reviewed Mar 25, 2026

View reviewed changes

fixup! modernize metadata helpers and async tests

1ac13d8

claude bot reviewed Mar 25, 2026

View reviewed changes

fixup! modernize metadata helpers and async tests

21a9df6

claude bot reviewed Mar 25, 2026

View reviewed changes

Conversation

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

Uh oh!

Uh oh!

bert-e commented Feb 23, 2026

Hello benzekrimaha,

Uh oh!

bert-e commented Feb 23, 2026

Incorrect fix version

Uh oh!

codecov bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bert-e commented Feb 23, 2026

Waiting for approval

Uh oh!

bert-e commented Feb 24, 2026

Incorrect fix version

Uh oh!

bert-e commented Feb 24, 2026

Incorrect fix version

Uh oh!

bert-e commented Feb 24, 2026

Waiting for approval

Uh oh!

francoisferrand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bert-e commented Feb 25, 2026

Incorrect fix version

Uh oh!

bert-e commented Feb 27, 2026

Request integration branches

Uh oh!

SylvainSenechal left a comment

Choose a reason for hiding this comment

Uh oh!

francoisferrand commented Mar 11, 2026

Uh oh!

francoisferrand Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

francoisferrand Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francoisferrand Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

claude bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

claude bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benzekrimaha commented Feb 23, 2026 •

edited

Loading

codecov bot commented Feb 23, 2026 •

edited

Loading

francoisferrand Mar 16, 2026 •

edited

Loading