CLDSRV-717: Move utils out of multi backend#5895
Conversation
The awslocation in multiBackend breaks S3C Integration
Hello bourgoismickael,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/9.0 #5895 +/- ##
================================================
Coverage 83.19% 83.19%
================================================
Files 188 188
Lines 12091 12091
================================================
Hits 10059 10059
Misses 2032 2032
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
anurag4DSB
left a comment
There was a problem hiding this comment.
Interesting way, could we have some developer comments for someone looking at the code to understand why it was done?
In Integration, the test |
|
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
|
/approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-717. Goodbye bourgoismickael. The following options are set: approve, create_integration_branches |
There was a problem hiding this comment.
Pull request overview
This PR extracts Mocha test “skip” helpers (Ceph / multiple-backend gating) into a dedicated utility module so non–multiple-backend tests no longer need to import the heavy test/multipleBackend/utils.js (which includes location constants like awsLocation) that can interfere with S3C integration.
Changes:
- Added
tests/functional/aws-node-sdk/lib/utility/test-utils.jsto host shared Ceph/multiple-backend skip helpers. - Updated
rangeTest.jsto importdescribeSkipIfCephfrom the new shared module instead oftest/multipleBackend/utils.js. - Updated
routeBackbeat.jsandtest/multipleBackend/utils.jsto consume the new shared helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/multipleBackend/routes/routeBackbeat.js | Switches skip-helper imports to the new shared test utility module. |
| tests/functional/aws-node-sdk/test/object/rangeTest.js | Avoids importing multiple-backend utils by sourcing describeSkipIfCeph from the new module. |
| tests/functional/aws-node-sdk/test/multipleBackend/utils.js | Removes inline skip-helper definitions and imports them from the new module. |
| tests/functional/aws-node-sdk/lib/utility/test-utils.js | New shared module defining Mocha skip helpers based on CI_CEPH and config.backends.data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const describeSkipIfCeph = isCEPH ? describe.skip : describe.skip; // always skip | ||
| let describeSkipIfNotMultiple = describe.skip; | ||
| let describeSkipIfNotMultipleOrCeph = describe.skip; | ||
|
|
||
| if (config.backends.data === 'multiple') { | ||
| describeSkipIfNotMultiple = describe; | ||
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe.skip; // always skip |
There was a problem hiding this comment.
describeSkipIfCeph is currently set to describe.skip in both branches of the ternary, so any suite using it will be skipped even when not running on Ceph. If the intent is to only skip on Ceph (as the name suggests), change the non-Ceph branch to describe (or rename this helper to reflect that it always skips).
| const describeSkipIfCeph = isCEPH ? describe.skip : describe.skip; // always skip | |
| let describeSkipIfNotMultiple = describe.skip; | |
| let describeSkipIfNotMultipleOrCeph = describe.skip; | |
| if (config.backends.data === 'multiple') { | |
| describeSkipIfNotMultiple = describe; | |
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe.skip; // always skip | |
| const describeSkipIfCeph = isCEPH ? describe.skip : describe; | |
| let describeSkipIfNotMultiple = describe.skip; | |
| let describeSkipIfNotMultipleOrCeph = describe.skip; | |
| if (config.backends.data === 'multiple') { | |
| describeSkipIfNotMultiple = describe; | |
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe; |
| const describeSkipIfCeph = isCEPH ? describe.skip : describe.skip; // always skip | ||
| let describeSkipIfNotMultiple = describe.skip; | ||
| let describeSkipIfNotMultipleOrCeph = describe.skip; | ||
|
|
||
| if (config.backends.data === 'multiple') { | ||
| describeSkipIfNotMultiple = describe; | ||
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe.skip; // always skip |
There was a problem hiding this comment.
describeSkipIfNotMultipleOrCeph is set to describe.skip even when config.backends.data === 'multiple' and not running on Ceph (both branches of the ternary are describe.skip). This makes every suite wrapped with this helper permanently skipped; if the intent is to run on multiple-backend non-Ceph setups, the non-Ceph branch should be describe.
| const describeSkipIfCeph = isCEPH ? describe.skip : describe.skip; // always skip | |
| let describeSkipIfNotMultiple = describe.skip; | |
| let describeSkipIfNotMultipleOrCeph = describe.skip; | |
| if (config.backends.data === 'multiple') { | |
| describeSkipIfNotMultiple = describe; | |
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe.skip; // always skip | |
| const describeSkipIfCeph = isCEPH ? describe.skip : describe; | |
| let describeSkipIfNotMultiple = describe.skip; | |
| let describeSkipIfNotMultipleOrCeph = describe.skip; | |
| if (config.backends.data === 'multiple') { | |
| describeSkipIfNotMultiple = describe; | |
| describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe; |
The awslocation in multiBackend breaks S3C Integration