Skip to content

Conversation

@SylvainSenechal
Copy link
Contributor

ISSUE: CLDSRV-819

As test files depend on these utilities,
they needed to be adapted to match the sdk v3
requirements.

Issue: CLDSRV-724
@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.25%. Comparing base (d761a3a) to head (21363b6).
⚠️ Report is 149 commits behind head on development/9.3.
✅ All tests successful. No failed tests found.

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

❌ Your patch status has failed because the patch coverage (33.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/routes/routeBackbeat.js 75.79% <33.33%> (-0.54%) ⬇️

... and 9 files with indirect coverage changes

@@                 Coverage Diff                 @@
##           development/9.3    #6042      +/-   ##
===================================================
- Coverage            84.39%   84.25%   -0.14%     
===================================================
  Files                  204      204              
  Lines                12928    12937       +9     
===================================================
- Hits                 10910    10900      -10     
- Misses                2018     2037      +19     
Flag Coverage Δ
file-ft-tests 67.41% <0.00%> (-0.02%) ⬇️
kmip-ft-tests 28.13% <0.00%> (-0.02%) ⬇️
mongo-v0-ft-tests 68.76% <0.00%> (+0.07%) ⬆️
mongo-v1-ft-tests 68.66% <0.00%> (-0.04%) ⬇️
multiple-backend 35.28% <0.00%> (-0.05%) ⬇️
sur-tests 35.63% <0.00%> (-0.83%) ⬇️
sur-tests-inflights 37.44% <0.00%> (-0.06%) ⬇️
unit 69.90% <33.33%> (-0.03%) ⬇️
utapi-v2-tests 34.29% <0.00%> (-0.27%) ⬇️

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.

@SylvainSenechal
Copy link
Contributor Author

Waiting for Arsenal release to bump

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This commit :

  • Leif's new feature server access logs : He added tests in ~november-december, using sdk v2 syntax, with a utility client that is now migrated to sdk v3 => I initialized a sdk v2 client directly for those tests, and will migrate to v3 in a separate ticket
  • Run this Pr in Zenko and fixed anything that was broken
  • Some nits comments last Maha's pr addressed (formatting, tabs etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are new tests for server access logs : using sdk v2 for now.
The function empty bucket that I added is basically a copy of the one that was in bucket utility (that we can't use because it's v3 compatible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file I don't remember exactly what happened, but I think some miss resolved conflicts or something like that

});


afterEach(done => {
removeAllVersions({ Bucket: bucketName }, err => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not keep the fully async implementation of this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removeAllVersions takes a callback. Not sure what happened here, I guess at some point there was an attempt at updating removeAllVersions to be async, but since it's an utility used ~100 times, it was eventually not done

const objects = objectsRes.slice(0, 1000).map(obj =>
({ Key: obj.Key, VersionId: obj.VersionId }));
return s3.deleteObjects({

Copy link
Contributor

Choose a reason for hiding this comment

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

some spaces at EOL in this file (here & below)

removeAllVersions({ Bucket: bucketName }, err => {
if (err) {
if (err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing one leve indent

Comment on lines 232 to 223
it('should send back a DeleteMarkerVersionId matching the versionId ' +
'stored for the object if trying to delete an object that does not exist',
done => {
s3.deleteObjects({ Bucket: bucketName,
async () => {
const deleteRes = await s3.send(new DeleteObjectsCommand({
Copy link
Contributor

@francoisferrand francoisferrand Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
it('should send back a DeleteMarkerVersionId matching the versionId ' +
'stored for the object if trying to delete an object that does not exist',
done => {
s3.deleteObjects({ Bucket: bucketName,
async () => {
const deleteRes = await s3.send(new DeleteObjectsCommand({
it('should send back a DeleteMarkerVersionId matching the versionId ' +
'stored for the object if trying to delete an object that does not exist', async () => {
const deleteRes = await s3.send(new DeleteObjectsCommand({

Copy link
Contributor

Choose a reason for hiding this comment

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

(same below)

return next(null, currentMD || md || {});
});
}

Copy link
Contributor

Choose a reason for hiding this comment

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

extra space at EOL

Last adjustements following tests run against cloudserver and Zenko CI
Update tests for server access log file to use sdk v2 for now
ISSUE: CLDSRV-819
@SylvainSenechal
Copy link
Contributor Author

/approve

@scality scality deleted a comment from bert-e Jan 9, 2026
@bert-e
Copy link
Contributor

bert-e commented Jan 9, 2026

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/9.3

The following branches will NOT be impacted:

  • development/7.10
  • development/7.4
  • development/7.70
  • development/8.8
  • development/9.0
  • development/9.1
  • development/9.2

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Jan 9, 2026

Queue build failed

The corresponding build for the queue failed:

  • Checkout the status page.
  • Identify the failing build and review the logs.
  • If no issue is found, re-run the build.
  • If an issue is identified, checkout the steps below to remove
    the pull request from the queue for further analysis and maybe rebase/merge.
Remove the pull request from the queue
  • Add a /wait comment on this pull request.
  • Click on login on the status page.
  • Go into the manage page.
  • Find the option called Rebuild the queue and click on it.
    Bert-E will loop again on all pull requests to put the valid ones
    in the queue again, while skipping the one with the /wait comment.
  • Wait for the new queue to merge, then merge/rebase your pull request
    with the latest changes to then work on a proper fix.
  • Once the issue is fixed, delete the /wait comment and
    follow the usual process to merge the pull request.

@bert-e
Copy link
Contributor

bert-e commented Jan 9, 2026

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/9.3

The following branches have NOT changed:

  • development/7.10
  • development/7.4
  • development/7.70
  • development/8.8
  • development/9.0
  • development/9.1
  • development/9.2

Please check the status of the associated issue CLDSRV-819.

Goodbye sylvainsenechal.

@bert-e bert-e merged commit 9d4f902 into development/9.3 Jan 9, 2026
28 of 29 checks passed
@bert-e bert-e deleted the improvement/CLDSRV-819 branch January 9, 2026 15:10
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.

6 participants